Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audio for Videos #1490

Merged
merged 12 commits into from
Dec 11, 2024
Merged

Audio for Videos #1490

merged 12 commits into from
Dec 11, 2024

Conversation

EricBAndrews
Copy link
Member

@EricBAndrews EricBAndrews commented Dec 9, 2024

Checklist

  • I have read CONTRIBUTING.md
  • I have described what this PR contains
  • If this PR alters the UI, I have attached pictures/videos
  • This PR addresses one or more open issues that were assigned to me:
    - closes Audio for Videos #1489

Pull Request Information

Enhances video handling:

  • mp4s can now play audio
  • Videos with audio now show a mute button on the top right
  • Default mute behavior can be controlled in settings (General -> Mute Videos)
  • Tapping videos now pauses them instead of stops them
  • mp4s now loop

Known Issues

@EricBAndrews EricBAndrews requested a review from a team as a code owner December 9, 2024 17:51
@EricBAndrews EricBAndrews requested review from JakeShirley and mormaer and removed request for a team December 9, 2024 17:51
@EricBAndrews EricBAndrews mentioned this pull request Dec 9, 2024
@EricBAndrews EricBAndrews requested a review from Sjmarf December 9, 2024 17:56
Copy link
Contributor

@Sjmarf Sjmarf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Pressing the volume up button whilst playing a muted video unmutes the AVQueuePlayer, but the mute button in the UI doesn't get updated. Maybe we need to watch player.muted for changes and update muted if so? Could be tricky though if a view update doesn't get triggered when player.muted changes.

@EricBAndrews
Copy link
Member Author

Ooh, nice catch! I've added some observation code that should sort it out.

@EricBAndrews EricBAndrews requested a review from Sjmarf December 9, 2024 23:20
Copy link
Contributor

@Sjmarf Sjmarf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! It would be nice if videos were paused once they leave the screen. If I unmute a video and scroll away, the audio keeps playing.

@EricBAndrews EricBAndrews enabled auto-merge (squash) December 11, 2024 03:38
@EricBAndrews EricBAndrews merged commit 59ce59d into dev Dec 11, 2024
2 checks passed
@EricBAndrews EricBAndrews deleted the eric/audio branch December 11, 2024 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audio for Videos
2 participants