Blake Wu [:bwu][:blakewu] Assignee
	Description • 10 years ago

==SW Version==
V2.2 Flame 

==STR==
1. Go to Youtube.
2. Search "HD movies". 
3. Play one of them.  
4. After playing, do seek and push home key.
5. Long press home key to go back the youtube. 

==Actual behavior==
Youtube will *not* resume. 

==Expected behavior==
Youtube will resume. 

==Repro rate==
Following the same steps, 50%.

If we don't do seek in step#4, this problem will not happen.

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 1 • 10 years ago

I can see this problem on V3.0 (repro rate, 30%). But the behavior of this problem is a little different. It will keep showing spinner and refreshing the browser still cannot work but show a black screen.

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 2 • 10 years ago

Sometimes mDecoder->Resume() is not called @ [1]

[1]https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp?from=HTMLMediaElement.cpp&case=true#3771

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 3 • 10 years ago

After some investigations, the rootcause is mMuted[1][2] is still muted by MUTED_BY_AUDIO_CHANNEL when time to resume since mPlayingThroughTheAudioChannel is false during seeking, so resume will not happen. 
From my logs, Youtube seems to do pause before seeking, so mPlayingThroughTheAudioChannel[3] is false during seeking. 

[1]https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp?from=HTMLMediaElement.cpp&case=true#4244
[2]https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp?from=HTMLMediaElement.cpp&case=true#3814
[3]https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp?from=HTMLMediaElement.cpp&case=true#4254

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 4 • 10 years ago

Currently media element(video and audio) suspend or resume mainly replies on the mechanism of audio channel. 
IMO, we should make video and audio work separately. 
Audio replies on mMuted based on current mechanism (audio channel).
If media element has video, it should refer to IsActive() and IsHidden() to make decision to suspend or resume.

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 5 • 10 years ago

Hi baku,
May I have your feedback?

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 6 • 10 years ago

(In reply to Blake Wu [:bwu][:blakewu] from comment #4)
> Currently media element(video and audio) suspend or resume mainly replies on
> the mechanism of audio channel. 
> IMO, we should make video and audio work separately. 
> Audio replies on mMuted based on current mechanism (audio channel).
> If media element has video, it should refer to IsActive() and IsHidden() to
> make decision to suspend or resume.
So if media element has video and it is active(IsActive) and not hidden (!IsHidden()), it should be resumed even mMuted is true for MUTED_BY_AUDIO_CHANNEL.

	Andrea Marchesini [:baku]
	Comment 7 • 10 years ago

Comment on attachment 8575178 [details] [diff] [review]
WIP-Bug-1123209-Unbind-audio-channel-from-video-case.patch

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

I disagree. With this patch we don't use the AudioChannel policy at all when there is a video. I guess this is not what we want because otherwise a simple video will play forever also if the mute the app.

A couple of questions:

1. why the video is muted? I remember that when we seek the video/audio, we consider it still active.
2. I'll try to reproduce the issue on a desktop build and see if I can reproduce it.

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 8 • 10 years ago

This bug is similar with what I found in bug 1147304 comment 4 quoted as below.

"The root cause of the problem described in comment 3 is 
In step 2, mPlayingThroughTheAudioChannel and mAudioChannelAgent will be set *false* and null respectively due to mReadyState is set to HAVE_METADATA in UpdateReadyStateForData[1]. So when backing to resume, since mPlayingThroughTheAudioChannel is false, SetVisibilityChange will not be called in NotifyOwnerDocumentActivityChanged[2] and the state of mMuted is still MUTED_BY_AUDIO_CHANNEL. Therefore, playback will not be resumed to play. 


[1]https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp?from=HTMLMediaElement.cpp&case=true#3534
[2]https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp?from=HTMLMediaElement.cpp&case=true#3960
"

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 9 • 10 years ago

(In reply to Andrea Marchesini (:baku) from comment #7)
> Comment on attachment 8575178 [details] [diff] [review]
> WIP-Bug-1123209-Unbind-audio-channel-from-video-case.patch
> 
> Review of attachment 8575178 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I disagree. With this patch we don't use the AudioChannel policy at all when
> there is a video. I guess this is not what we want because otherwise a
> simple video will play forever also if the mute the app.
What cases/situations video should not be played if it is active and not hidden? 
> 
> A couple of questions:
> 
> 1. why the video is muted? I remember that when we seek the video/audio, we
> consider it still active.
There is a very easy way (bug 1147304 comment 3, repro rate: 60%) to repro this problem without seek. The reason why audio is muted is as mentioned in comment 8. That usually happens when video element goes to dormant state which will release video codec and destroy the decoding pipeline. When resuming from dormant, it will start from ReadMetadata and prepare the decoding pipeline. It looks like current dormant policy and audio channel policy is not alignment, so in some cases, video element leaves dormant state but still in paused state due to audio channel policy. Therefore video playback doesn't play.

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 10 • 10 years ago

[Blocking Requested - why for this release]:
1. User can easily hit this problem. 
2. User experience is not good. User cannot watch youtube normally.

	Andrea Marchesini [:baku]
	Comment 12 • 9 years ago

We are landing the new AudioChannel policy. This patch is not needed anymore.

	Blake Wu [:bwu][:blakewu] Assignee
	Comment 13 • 9 years ago

(In reply to Andrea Marchesini (:baku) from comment #12)
> We are landing the new AudioChannel policy. This patch is not needed anymore.
Could you let me know which bug is for the new AudioChannel policy?
Thanks!

	Andrea Marchesini [:baku]
	Comment 14 • 9 years ago

bug 1113086.