1287116 - Once Video is suspended by playing video in the Youtube app, it can't be restarted (and no video works)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect, P1)

Description

[Mike Kaply [:mkaply]]

Note this is working on Firefox 50, but this problem was reported by a partner and we need something on Firefox 48 (plus Firefox 48 is very broke).

If you go to Firefox and start a video playing on Youtube, then go to youtube and start a video, Firefox pauses the video properly.

If you then go back to Firefox, you can't start the video again.

From then on, no videos work in Firefox at all. You have to restart the app.

I've tested on Android 6 and Android N.

This is beta 6.

Comment 1

[Mike Kaply [:mkaply]]

I verified this is broken with the very latest code.

I also checked the log and there are no notifications related to video/audio focus.

Comment 2

[Mike Kaply [:mkaply]]

I was searching for the wrong Log Tag. Here's the log

23723:mozilla-beta michaelkaply$ adb logcat | grep AudioFocusAgent
07-15 11:15:42.870 27309 27327 D AudioFocusAgent: NotifyStartedPlaying
07-15 11:15:42.872  2387  3930 I MediaFocusControl:  AudioFocus  requestAudioFocus() from uid/pid 10112/27309 clientId=[e-mail]$1@28fb565 req=1 flags=0x0
07-15 11:15:42.873 27309 27327 D AudioFocusAgent: AudioFocus request granted
07-15 11:15:58.201 27309 27327 D AudioFocusAgent: NotifyStoppedPlaying
07-15 11:15:58.201 27309 27327 D AudioFocusAgent: Abandon AudioFocus
07-15 11:15:58.202  2387 14166 I MediaFocusControl:  AudioFocus  abandonAudioFocus() from uid/pid 10112/27309 clientId=[e-mail]$1@28fb565
07-15 11:15:59.003 27309 27327 D AudioFocusAgent: NotifyStartedPlaying
07-15 11:15:59.003  2387 31392 I MediaFocusControl:  AudioFocus  requestAudioFocus() from uid/pid 10112/27309 clientId=[e-mail]$1@28fb565 req=1 flags=0x0
07-15 11:15:59.003 27309 27327 D AudioFocusAgent: AudioFocus request granted
07-15 11:16:10.214 27309 27309 D AudioManager: AudioManager dispatching onAudioFocusChange(-1) for [e-mail]$1@28fb565
07-15 11:16:10.215 27309 27309 D AudioFocusAgent: onAudioFocusChange, AUDIOFOCUS_LOSS
07-15 11:16:21.711 27309 27327 D AudioFocusAgent: NotifyStoppedPlaying
07-15 11:16:21.711 27309 27327 D AudioFocusAgent: Abandon AudioFocus
07-15 11:16:21.712  2387 14168 I MediaFocusControl:  AudioFocus  abandonAudioFocus() from uid/pid 10112/27309 clientId=[e-mail]$1@28fb565
07-15 11:16:21.713  2387 14168 I MediaFocusControl: AudioFocus  removeFocusStackEntry(): removing entry for [e-mail]$1@28fb565
07-15 11:16:21.801 27309 27327 D AudioFocusAgent: NotifyStartedPlaying
07-15 11:16:21.802  2387  3833 I MediaFocusControl:  AudioFocus  requestAudioFocus() from uid/pid 10112/27309 clientId=[e-mail]$1@28fb565 req=1 flags=0x0
07-15 11:16:21.803 27309 27327 D AudioFocusAgent: AudioFocus request granted

After that no video will play.

I continue to get notifications, but it simply doesn't work.

Comment 3

[Mike Kaply [:mkaply]]

Interesting behavior. I got a notification with sound and when that happened, the video started playing. This was five minutes later.

On the console I saw:

7-15 11:17:35.736 27309 27327 D AudioFocusAgent: AudioFocus request granted
07-15 11:20:01.356 27309 27309 D AudioManager: AudioManager dispatching onAudioFocusChange(-3) for [e-mail]$1@28fb565
07-15 11:20:03.429 27309 27309 D AudioManager: AudioManager dispatching onAudioFocusChange(1) for [e-mail]$1@28fb565
07-15 11:20:03.429 27309 27309 D AudioFocusAgent: onAudioFocusChange, AUDIOFOCUS_GAIN
07-15 11:20:09.646 27309 27327 D AudioFocusAgent: NotifyStoppedPlaying
07-15 11:20:09.646 27309 27327 D AudioFocusAgent: Abandon AudioFocus
07-15 11:20:09.647  2387  3770 I MediaFocusControl:  AudioFocus  abandonAudioFocus() from uid/pid 10112/27309 clientId=[e-mail]$1@28fb565

Looking back through the log, in the switching cases, we aren't getting AUDIOFOCUS_GAIN...

Comment 4

[Barbara Bermes [:barbara] - NI please!]

Kevin, can you help us verify this?

Comment 5

[Alastor Wu [:alwu]]

It was known issue when I developed the media control, and the bug1242874 was exactly used to fix this problem.
However, I don't encourage to uplift the bug1242874, it's too risky, because we have several bugs related with the Fennec media control. If we do so, that means we also need to uplift other bugs. Other bugs are all landed in FF49, so the FF49 doesn't have this issue.

My suggestion is that we can disable this feature only on FF48.
How do you guys think?

Comment 6

[Alastor Wu [:alwu]]

(In reply to Mike Kaply [:mkaply] from comment #1)
> I verified this is broken with the very latest code.
> 
> I also checked the log and there are no notifications related to video/audio
> focus.

I can't reproduce it on nightly, which version do you use?

Comment 7

[Mike Kaply [:mkaply]]

Sorry I meant latest beta,  not just b6. Is there really nothing we can do just to reenable the control?

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Alastor Wu [:alwu][PTO 7/20-7/28] from comment #5)
> My suggestion is that we can disable this feature only on FF48.
> How do you guys think?

What exactly do you want to disable? Can we fix this by just disabling a feature? Sounds like a good workaround. Are there any drawbacks?

Would it even be possible to uplift bug 1242874 without the follow-up bugs (remote controls etc.)? Because there have been a bunch of follow-ups and we are still fixing one (bug 1282391).

Comment 9

[Alastor Wu [:alwu]]

(In reply to Mike Kaply [:mkaply] from comment #7)
> Sorry I meant latest beta,  not just b6. Is there really nothing we can do
> just to reenable the control?

Other solution is to uplift the bug 1242874, but I think it's too risky.

(In reply to Sebastian Kaspari (:sebastian) from comment #8)
> What exactly do you want to disable? Can we fix this by just disabling a
> feature? Sounds like a good workaround. Are there any drawbacks?
> 
> Would it even be possible to uplift bug 1242874 without the follow-up bugs
> (remote controls etc.)? Because there have been a bunch of follow-ups and we
> are still fixing one (bug 1282391).

Yes, we can only disable this feature without any side effect.
Even we skip the media control parts, we still have several bugs related with the platform behaviors (eg. play-pause, audio competing in tab, audible checking .e.t.c.)

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1287116 - [workaround] disable audio focus on FF48.

This changeset should only be applied to FF48, it's a workaround.

Review commit: https://reviewboard.mozilla.org/r/64960/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64960/

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8772027 [details]
Bug 1287116 - [workaround] disable audio focus on FF48.

Okay, that's not particular nice but if this works then this is okay for me. It'll be gone with 49.

@mkaply: Can you build this and verify that this fixes the issues the partner is seeing?

Comment 12

[Mike Kaply [:mkaply]]

No, the workaround doesn't work for me. Video still won't resume on Fennec. I also tried returning from OnAudioFocusChangeListener and that didn't work either.

I uninstalled the beta completely to make sure there were no remnants around.

Comment 13

[Mike Kaply [:mkaply]]

Backing out the second patch from bug 1249579 took me back to the old behavior where the two audio streams play simultaneously (which I'd argue is better than the Firefox audio suspending and not playing).

The partner won't be happy with either of these scenarios, but I think this is the lesser of two evils.

Is there really not some simple all we can make when we receive focus to just get audio playing again (without all the controls stuff)?

Comment 14

[Alastor Wu [:alwu]]

(In reply to Mike Kaply [:mkaply] from comment #13)
> Backing out the second patch from bug 1249579 took me back to the old
> behavior where the two audio streams play simultaneously (which I'd argue is
> better than the Firefox audio suspending and not playing).
> 
> The partner won't be happy with either of these scenarios, but I think this
> is the lesser of two evils.
> 
> Is there really not some simple all we can make when we receive focus to
> just get audio playing again (without all the controls stuff)?

As comment 9 I said, the control stuff isn't the most important point.
The point is we have several platform side behaviors related with audio focus, so I don't know whether we can uplift the bug1242874 without any mistake. 

But if you really want to keep this feature on FF48, you can try to apply the bug1242874 to your build.

Comment 15

[Mike Kaply [:mkaply]]

I'm honestly not sure what the right thing to do is here.

Bug 1249579 is nice in that it pauses audio, but if can't resume, it seems like we'll get lots of bug reports.

I'm wondering if we should just back 1249579 out of Firefox 48.

Comment 16

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: Disabling should be fine, it is equivalent to backing out. We are 2 weeks from release. We should confirm that Aurora 49.0a2 works and this fix behaves as expected. Since this is close to the release NI to Sylvestre for visibility.

Comment 17

[Mike Kaply [:mkaply]]

I have verified that 49.0a2 works great. Focus changes cause audio to start and stop, and you can resume audio in the browser.

Should I investigate backing out bug 1249579 or some small change that simply turns it off?

Does bug 1249579 do anything important without the other changes?

Comment 18

[Kevin Brosnan [Ex-Mozilla]]

The patch here is a simple if true statement, I would lean towards that over a backout.

Comment 19

[Mike Kaply [:mkaply]]

> The patch here is a simple if true statement, I would lean towards that over a backout.

The patch didn't work for me. I'm continuing to investigate.

Comment 20

[Mike Kaply [:mkaply]]

i just did a full build and the patch does work. I'd like to request uplift to 48.

Comment 21

[Gerry Chang [:gchang]]

Track this as this issue has impact on users for watching video.

Comment 22

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8772027 [details]
Bug 1287116 - [workaround] disable audio focus on FF48.

https://reviewboard.mozilla.org/r/64960/#review62522

Comment 23

[Alastor Wu [:alwu]]

Comment on attachment 8772027 [details]
Bug 1287116 - [workaround] disable audio focus on FF48.

Approval Request Comment
[Feature/regressing bug #]: disable audio focus
[User impact if declined]: the video would not be playback again after audio competing happens
[Describe test coverage new/current, TreeHerder]: no
[Risks and why]: no
[String/UUID change made/needed]:no

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8772027 [details]
Bug 1287116 - [workaround] disable audio focus on FF48.

Looks like we have no other choice. Should be in 48 beta 10.

By the way, every patch has risk. "no" is not a valid option ;)

Comment 25

[Carsten Book [:Tomcat]]

has problems applying to beta:

Tomcats-MacBook-Pro-2:mozilla-beta Tomcat$ hg import https://reviewboard-hg.mozilla.org/gecko/rev/7a63f87b88b1f0512ae6ade8bc8564f64cc144b8
applying https://reviewboard-hg.mozilla.org/gecko/rev/7a63f87b88b1f0512ae6ade8bc8564f64cc144b8
patching file mobile/android/base/java/org/mozilla/gecko/media/AudioFocusAgent.java
Hunk #1 FAILED at 96
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/java/org/mozilla/gecko/media/AudioFocusAgent.java.rej

Comment 26

[Sebastian Kaspari (:sebastian; :pocmo)]

Alastor is on PTO. If I can find time I'll try to fix the patch.

Comment 27

[Mike Kaply [:mkaply]]

Attachment: New patch based on mozilla-beta

Here's a new patch that applies to beta.

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b8a75eb3cacc

I'll leave it to you to resolve the bug if that's it for here. :)

Comment 29

[Sorina Florean [:sflorean]]

Verified as fixed in Firefox 48 Beta 10 with steps from comment 0;
Device: Nexus 5 (Android 6.0.1).

Comment 30

[Mike Kaply [:mkaply]]

Attachment: Fix problem (not just disable).

I had been continuing to look at this and come to find out, there is an easy one line fix.

This brings the functionality back and allows the resuming of the video.

We need this for FF48 as it's a better fix for the partner than disabling.

Comment 31

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8774409 [details] [diff] [review]
Fix problem (not just disable).

Review of attachment 8774409 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable. r+ so that you can land it. I'll flag alastor for feedback, just so that he can take a look once he's back.

Comment 32

[Mike Kaply [:mkaply]]

Comment on attachment 8774409 [details] [diff] [review]
Fix problem (not just disable).

Approval Request Comment
[Feature/regressing bug #]: 1249579 - We had turned off audio focus because it caused a bad resume experience.
[User impact if declined]: When a phone call arrives, audio continues to play.
[Describe test coverage new/current, TreeHerder]: Tested locally. Verified
[Risks and why]: Low. Previous code had been enabled for a while. This adds one new codepath.
[String/UUID change made/needed]: None

Comment 33

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8774409 [details] [diff] [review]
Fix problem (not just disable).

[Triage Comment]
partner stuff: let's take it on all branches!

Comment 34

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/42a797ef3647
https://hg.mozilla.org/releases/mozilla-release/rev/fd1e1d7cfb68
This doesn't apply cleanly to aurora or m-c, Mike said that was fine on IRC.

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8774409 [details] [diff] [review]
Fix problem (not just disable).

we don't need it in aurora.

Comment 36

[Sorina Florean [:sflorean]]

Tested using:
Device: Nexus 5 (Android 6.0.1)
Build: Firefox 48 Beta 11 (Build 1)
This was verified again and confirmed as correctly implemented.

Comment 37

[Alastor Wu [:alwu]]

Comment on attachment 8774409 [details] [diff] [review]
Fix problem (not just disable).

Does this patch work for this issue?

According the steps in the description, I think this changeset wouldn't be called when we go back to Firefox.

Comment 38

[Mike Kaply [:mkaply]]

> Does this patch work for this issue?

> According the steps in the description, I think this changeset wouldn't be called when we go back to Firefox.

It worked in my testing on two different devices. Whereas before after switching back, the video simply had a the spinner and no amount of clicking would resume the video, after the patch you could play the video again.

Comment 39

[Alastor Wu [:alwu]]

(In reply to Mike Kaply [:mkaply] from comment #38)
> It worked in my testing on two different devices. Whereas before after
> switching back, the video simply had a the spinner and no amount of clicking
> would resume the video, after the patch you could play the video again.

OK, It's fine to me if this patch would only be landed in FF48.