Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.91k forks source link

unstable sound levels after unmuting the video #52858

Open m-natarajan opened 1 day ago

m-natarajan commented 1 day ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.64-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @sumo-slonik Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs

Action Performed:

  1. Upload or send a video file as attachment
  2. Play the video
  3. Mute and unmute the volume

    Expected Result:

    The volume level is stable

    Actual Result:

    The volume level is unstable

    Workaround:

    Unknown

    Platforms:

    Which of our officially supported platforms is this issue occurring on?

    • [ ] Android: Standalone
    • [ ] Android: HybridApp
    • [ ] Android: mWeb Chrome
    • [x] iOS: Standalone
    • [x] iOS: HybridApp
    • [ ] iOS: mWeb Safari
    • [ ] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/7e52428f-3e37-4db0-8546-f41616b10329

https://github.com/user-attachments/assets/bfedfbc3-4a35-449e-aa1b-e21bb2ff5028

View all open jobs on GitHub

melvin-bot[bot] commented 1 day ago

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

bernhardoj commented 1 day ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Unstable state of the video sound level.

What is the root cause of that problem?

When we press the sound to unmute, we update the volume to 1 and set the isMuted to false. https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/components/VideoPlayerContexts/VolumeContext.tsx#L13-L20

But in BaseVideoPlayer, inside the playback status update listener, we update the volume to 0.25 if the volume is previously 0 and it's previously muted. https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/components/VideoPlayer/BaseVideoPlayer.tsx#L214-L219

That logic was added in https://github.com/Expensify/App/issues/45277. The root cause is explained on the selected proposal, but basically, when in fullscreen mode, pressing the sound to unmute it will do nothing. It's because the button changes the isMuted to false, but the volume is still 0. So, the above logic was added to change the volume to 0.25 (not sure why 0.25 and not 1).

What changes do you think we should make in order to solve the problem?

We need to apply that logic only if we are on fullscreen mode. We already did that for the 2nd if above.

if (isFullScreenRef.current) {
    if (prevIsMutedRef.current && prevVolumeRef.current === 0 && !status.isMuted) {
        updateVolume(0.25);
    }
    if (prevVolumeRef.current !== 0 && status.volume === 0 && !status.isMuted) {
        currentVideoPlayerRef.current?.setStatusAsync({isMuted: true});
    }
}
blazejkustra commented 1 day ago

@bernhardoj This issue doesn't have help wanted tag yet, so no proposal needed.

@sumo-slonik reported this issue and we believe having hardcoded 0.25 value is very unusual. Rather than adding more logic we would like to simplify this so it is less prone to bugs in a long run.

blazejkustra commented 1 day ago

We found this bug while investigating Reanimated usage in the app (we are both from Software Mansion). @zanyrenney if you agree that we want to fix this bug and the issue gets 'Help Wanted', please consider assigning @sumo-slonik 😄

sumo-slonik commented 1 day ago

Hey! I'm Kuba Nowakowski from Software Mansion, an expert agency, and I'd like to work on this issue!