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

[HOLD for payment 2024-06-06] [$250] IOS - Video - Video sound is not playing when device sound profile is silent mode #41204

Closed
1 of 7 tasks
kbecciv opened this issue Apr 29, 2024 · 36 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Apr 29, 2024

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: 1.4.67-0
  • Reproducible in staging?: y
  • Reproducible in production?: y
  • Issue reported by: Applause - Internal Team

Action Performed:

Precondition: Device sound mode is not silent mode.

  1. Launch New Expensify app.
  2. Go to chat.
  3. Upload a video with sound.
  4. Play the video.
  5. Note that the sound is playing.
  6. Change the device sound mode to silent mode.
  7. Play the video.

Expected Result:

The video sound should play as long as the media volume is not zero.

Actual Result:

The video sound is not playing when device sound profile is silent mode.

Workaround:

n/a

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
  • Screenshots/Videos

Add any screenshot/video evidence

Bug6464967_1714383898597.RPReplay_Final1714383538__1_.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @abekkala
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 29, 2024
Copy link

melvin-bot bot commented Apr 29, 2024

Triggered auto assignment to @abekkala (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.

@kbecciv
Copy link
Author

kbecciv commented Apr 29, 2024

We think that this bug might be related to #vip-vsb

@kbecciv
Copy link
Author

kbecciv commented Apr 29, 2024

@abekkala FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

@ShridharGoel
Copy link
Contributor

Proposal

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

Video sound is not playing when device sound profile is silent mode in iOS.

What is the root cause of that problem?

This is the default functionality of expo-av in iOS.

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

Add the below in Expensify.tsx. This will enable media volume in silent mode for iOS.

useEffect(() => {
    Audio.setAudioModeAsync({ playsInSilentModeIOS: true })
}, []);

We can add in BaseVideoPlayer instead, but Expensify.tsx would be a better choice.

What alternative options did you explore?

We can update the AppDelegate.m file to add the below line in didFinishLaunchingWithOptions.

[[AVAudioSession sharedInstance] setCategory:AVAudioSessionCategoryPlayback error:nil];

@melvin-bot melvin-bot bot added the Overdue label May 1, 2024
Copy link

melvin-bot bot commented May 2, 2024

@abekkala Whoops! This issue is 2 days overdue. Let's get this updated quick!

@abekkala abekkala added the External Added to denote the issue can be worked on by a contributor label May 2, 2024
@melvin-bot melvin-bot bot changed the title IOS - Video - Video sound is not playing when device sound profile is silent mode [$250] IOS - Video - Video sound is not playing when device sound profile is silent mode May 2, 2024
Copy link

melvin-bot bot commented May 2, 2024

Job added to Upwork: https://www.upwork.com/jobs/~018b7c5dab39a2df1f

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 2, 2024
Copy link

melvin-bot bot commented May 2, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

@melvin-bot melvin-bot bot removed the Overdue label May 2, 2024
@abekkala
Copy link
Contributor

abekkala commented May 2, 2024

@getusha we do have one proposal already for this one!

@getusha
Copy link
Contributor

getusha commented May 3, 2024

@abekkala @kbecciv is there a reproduction step? is it only on iOS?

@kbecciv
Copy link
Author

kbecciv commented May 6, 2024

@abekkala Can you please bring the OP back? Thank you!

@melvin-bot melvin-bot bot added the Overdue label May 6, 2024
Copy link

melvin-bot bot commented May 7, 2024

@abekkala, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@abekkala
Copy link
Contributor

abekkala commented May 8, 2024

@abekkala Can you please bring the OP back? Thank you!

Odd, yes I put it back!

@abekkala
Copy link
Contributor

abekkala commented May 8, 2024

@getusha OP is back - @kbecciv reported this as affecting iOS: Native

Copy link

melvin-bot bot commented May 9, 2024

@abekkala, @getusha Huh... This is 4 days overdue. Who can take care of this?

@getusha
Copy link
Contributor

getusha commented May 9, 2024

Will try reproducing this & we have a proposal to review

@melvin-bot melvin-bot bot removed the Overdue label May 9, 2024
@getusha
Copy link
Contributor

getusha commented May 12, 2024

Asked another c+ with a physical iOS device https://expensify.slack.com/archives/C02NK2DQWUX/p1715517674210059
@c3024 will take it.

@getusha getusha removed their assignment May 12, 2024
@c3024
Copy link
Contributor

c3024 commented May 13, 2024

@ShridharGoel 's proposal here looks good to me.

🎀 👀 🎀 C+ Reviewed

Copy link

melvin-bot bot commented May 13, 2024

Triggered auto assignment to @MonilBhavsar, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@MonilBhavsar
Copy link
Contributor

Is this a feature and not a bug?

@c3024
Copy link
Contributor

c3024 commented May 15, 2024

It is a feature of iOS native.

But this iOS native behaviour is different from Android native, Android Chrome and iOS Safari. In these three, video sound continues to play even after changing the device to silent mode. This inconsistency can be considered as a bug.

@MonilBhavsar
Copy link
Contributor

Thanks for clarifying!

@c3024
Copy link
Contributor

c3024 commented May 17, 2024

@ShridharGoel any update on this?

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels May 18, 2024
@ShridharGoel
Copy link
Contributor

@c3024 #42366

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels May 30, 2024
@melvin-bot melvin-bot bot changed the title [$250] IOS - Video - Video sound is not playing when device sound profile is silent mode [HOLD for payment 2024-06-06] [$250] IOS - Video - Video sound is not playing when device sound profile is silent mode May 30, 2024
Copy link

melvin-bot bot commented May 30, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label May 30, 2024
Copy link

melvin-bot bot commented May 30, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.77-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-06-06. 🎊

For reference, here are some details about the assignees on this issue:

  • @ShridharGoel requires payment (Needs manual offer from BZ)
  • @c3024 requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented May 30, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@c3024] The PR that introduced the bug has been identified. Link to the PR:
  • [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@c3024] Determine if we should create a regression test for this bug.
  • [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@abekkala
Copy link
Contributor

abekkala commented Jun 6, 2024

PAYMENT SUMMARY FOR JUN 06

@c3024
Copy link
Contributor

c3024 commented Jun 6, 2024

The PR that introduced the bug has been identified. Link to the PR:

No specific PR can be made responsible for this. This is a specific iOS behaviour.

The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

NA

A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

No discussion was started as this is a particular iOS specific behaviour and cannot be identified earlier.

Determine if we should create a regression test for this bug.

Yes

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

  1. Switch iOS device to silent mode.
  2. Keep the media volume on.
  3. Go to any chat and play a video with audio.
  4. Verify that the sound is audible.

👍 or 👎

@c3024
Copy link
Contributor

c3024 commented Jun 6, 2024

@abekkala thanks, accepted.

@abekkala
Copy link
Contributor

abekkala commented Jun 6, 2024

@c3024 - payment sent and contract ended - thank you! 🎉

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 6, 2024

This comment was marked as duplicate.

@abekkala
Copy link
Contributor

abekkala commented Jun 7, 2024

@ShridharGoel can you please accept the offer in Upwork so that I can submit payment and close this one? Offer Link thanks!

@melvin-bot melvin-bot bot added the Overdue label Jun 10, 2024
@abekkala
Copy link
Contributor

@ShridharGoel payment sent and contract ended - thank you! 🎉

@melvin-bot melvin-bot bot removed the Overdue label Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Archived in project
Development

No branches or pull requests

7 participants