Closed Bug 1347648 Opened 3 years ago Closed 3 years ago

Firefox requests audio focus for no-audio media (e.g. GIFV)

Categories

(Firefox for Android :: Audio/Video, enhancement, P2)

All
Android
enhancement

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
fennec + ---
firefox52 --- wontfix
firefox53 --- wontfix
firefox54 --- verified
firefox55 --- verified

People

(Reporter: JanH, Assigned: alwu)

References

()

Details

Attachments

(5 files)

We shouldn't do this - users might have music/podcasts/... playing in some other app and requesting audio focus for no-audio content (e.g. GIFV videos) does nothing but unnecessarily interrupt this.
Hi Jan.
May I know which app/page you you get this issue?
oops.. Sorry. it's the GIFV files. Maybe related to code here : http://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java#300
Flags: needinfo?(jh+bugzilla)
Hey Alastor,
Any idea about this?
tracking-fennec: ? → +
Flags: needinfo?(alwu)
Priority: -- → P2
Assignee: nobody → alwu
Flags: needinfo?(alwu)
Apparently you've already found an example file.
Flags: needinfo?(jh+bugzilla)
Comment on attachment 8849968 [details]
Bug 1347648 - part2 : paused media element is non-audible.

https://reviewboard.mozilla.org/r/122730/#review125312
Attachment #8849968 - Flags: review?(jwwang) → review+
Comment on attachment 8850338 [details]
Bug 1347648 - part4 : notify audible change after finishing set suspend and pause state.

https://reviewboard.mozilla.org/r/122962/#review125316
Attachment #8850338 - Flags: review?(jwwang) → review+
Comment on attachment 8849967 [details]
Bug 1347648 - part1 : only request Android audio focus for audible media.

https://reviewboard.mozilla.org/r/122704/#review125426
Attachment #8849967 - Flags: review?(snorp) → review+
Comment on attachment 8850275 [details]
Bug 1347648 - part3 : only request audio focus from gecko.

https://reviewboard.mozilla.org/r/122904/#review125906
Attachment #8850275 - Flags: review?(s.kaspari) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 8e327cbac38e -d 1bfd5d58b3b1: rebasing 384149:8e327cbac38e "Bug 1347648 - part1 : only request Android audio focus for audible media. r=snorp"
rebasing 384150:2222db14a5d2 "Bug 1347648 - part2 : paused media element is non-audible. r=jwwang"
merging dom/html/HTMLMediaElement.cpp
rebasing 384151:a8ff33567602 "Bug 1347648 - part3 : only request audio focus from gecko. r=sebastian"
rebasing 384152:48335ced306a "Bug 1347648 - part4 : notify audible change after finishing set suspend and pause state. r=jwwang" (tip)
merging dom/html/HTMLMediaElement.cpp
warning: conflicts while merging dom/html/HTMLMediaElement.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 77664f4f12b8 -d 1bfd5d58b3b1: rebasing 384153:77664f4f12b8 "Bug 1347648 - part1 : only request Android audio focus for audible media. r=snorp"
rebasing 384154:362d440b50a5 "Bug 1347648 - part2 : paused media element is non-audible. r=jwwang"
merging dom/html/HTMLMediaElement.cpp
rebasing 384155:ee2f0f2c205c "Bug 1347648 - part3 : only request audio focus from gecko. r=sebastian"
rebasing 384156:808e9ca1aa31 "Bug 1347648 - part4 : notify audible change after finishing set suspend and pause state. r=jwwang" (tip)
merging dom/html/HTMLMediaElement.cpp
warning: conflicts while merging dom/html/HTMLMediaElement.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/29ad09fc4655
part1 : only request Android audio focus for audible media. r=snorp
https://hg.mozilla.org/integration/autoland/rev/dbcbc70b0bc7
part2 : paused media element is non-audible. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/ff66592a8baf
part3 : only request audio focus from gecko. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/e8a7790bd268
part4 : notify audible change after finishing set suspend and pause state. r=jwwang
Uplift flag.
Flags: needinfo?(alwu)
Hmm - working as expected with regards to external apps, but still interrupts playback from other tabs *within* Firefox. Opening a separate bug for that, though…
Blocks: 1351087
Approval Request Comment
[Feature/Bug causing the regression]: Not a regression
[User impact if declined]: Playing non-audible media on Fennec would interrupt other playing app and stop them.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: Would only happen when user play non-audible media and other app is playing, won't affect the basic ability of Fennec
[String changes made/needed]: No
Flags: needinfo?(alwu)
Attachment #8851906 - Flags: approval-mozilla-beta?
Attachment #8851906 - Flags: approval-mozilla-aurora?
(In reply to Alastor Wu [:alwu] from comment #34)
> [Feature/Bug causing the regression]: Not a regression

Hmmm, since the audio focus/media control work is still quite fresh, I think that's debatable at the very least. A few versions ago we didn't even request audio focus at all (bug 1249579). Without running mozregression I can't say offhand which bug actually caused this (the initial implementation in bug 1249579 might have been completely fine, it might have been something related to the media control notification instead), but at some point between then and now *something* did change our behaviour here.
Jan or Alastor, did it interrupt playback from other tabs before landing this fix? 

I don't want to take this big of a change on beta without much time left for testing. We might still be able to uplift to aurora but I will leave that up to Gerry.
Flags: needinfo?(jh+bugzilla)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #36)
> Jan or Alastor, did it interrupt playback from other tabs before landing
> this fix? 

Yes, that is intended to do.
Depends on: 1351615
@Liz:
Yup, that behaviour happened before this patch as well.
Flags: needinfo?(jh+bugzilla)
Attachment #8851906 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8851906 [details] [diff] [review]
Bug1347648 - don't request audio focus for non-audible media (aurora/beta)

Fix an issue that playing non-audible media on Fennec would interrupt other playing app. Aurora54+.
Attachment #8851906 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on Beta build 54.0b2.
Device: Nexus 6 (Android 7.0).
You need to log in before you can comment on or make changes to this bug.