Closed Bug 1342913 Opened 7 years ago Closed 7 years ago

When stream ID change during internal seeking, waiting promise will never be resolved.

Categories

(Core :: Audio/Video: Playback, defect)

Product:
Core
Component:
Audio/Video: Playback
Version:
51 Branch
Platform:
x86_64
Windows 10
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr45 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix
firefox-esr52 55+ fixed
firefox53 + fixed
firefox54 + fixed
firefox55 --- fixed

People

(Reporter: sausharm, Assigned: jya)

References

Details

Attachments

(2 files)

Bug 1342913: P1. Add mochitest.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1342913: P2. Terminate draining operations when possible.
59 bytes, text/x-review-board-request
mozbugz
: review+
gchang
: approval-mozilla-aurora+
gchang
: approval-mozilla-beta+
gchang
: approval-mozilla-esr52+
Details 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

Will share repro steps shortly.


Actual results:

video stucks in buffering indefinitely even if video's current time is between video's buffered range.
document.getElementsByTagName('video')[0].currentTime
6.063628
document.getElementsByTagName('video')[0].buffered.start(0);
0.079588
document.getElementsByTagName('video')[0].buffered.end(0);
12



Expected results:

video should play fine till end of buffered range data.(i.e. 12 sec)
Also, it works fine on chrome(56.0.2924.87) browser.
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
the report as it is now is unusable...
Flags: needinfo?(sausharm)
Steps to reproduce:

I created a script (firefoxMSEBug.html) that is simulating the repro steps.
So, to reproduce particular issue just load the following URL.
http://de9b7h88wgj5l.cloudfront.net/static/assets/firefox-bug/firefoxMSEBug.html 


Actual results:

video stucks in buffering indefinitely even if video's current time is between video's buffered range.
document.getElementsByTagName('video')[0].currentTime
6.063628
document.getElementsByTagName('video')[0].buffered.start(0);
0.079588
document.getElementsByTagName('video')[0].buffered.end(0);
12



Expected results:

video should play fine till end of buffered range data.(i.e. 12 sec)
Also, it works fine on chrome(56.0.2924.87) browser.
Flags: needinfo?(sausharm)
works for me in Firefox 54. I notice no particular difference between the behaviour of Chrome and FF.

This loads data, wait a bit and seek to currentTime = 6 and then play the remaining time from 6 to 12s.
WFM in Firefox 52 and 51 too.

All those tested on mac. Will test on windows too.
I am able to reproduce this issue on firefox 51.0.1 (32-bit) on windows 10
yes, can reproduce on windows..

Another oddity with the Windows H264 MFT :(
When we can expect this fix? i.e. on which version of firefox this fix will be available?
that's assuming it can be fixed...

The issue is obviously in the Windows H264 MFT of which we have no control of.

So we need to find a workaround first.

It won't be in FF 52 that's certain as it's too late.
Assignee: nobody → jyavenard
please update this bug whenever there is an update on this.
This bug is very important for us.
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:


Oh...

this is a beauty... A condition not handled in the decoding state machine...

The first appendBuffer adds data from [0.079588, 6.068055) then it seeks to currentTime = 6.

Seeks complete and starts playback and stalls once reaching to currentTime = 6.06

A few seconds later, another appendBuffer is done, with data from [6.070333, 12.058800) from a different stream (including new init segment).

We are in waiting for data mode, the detection of stream is handled and the decoder is drained.

However, the drain never completes because it was assume that there would be never any change of it during internal seeking.


[Tracking Requested - why for this release]: Hoping that a fix could be done in a point release. This could explain some intermittents stall we see with various content.
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox-esr45: --- → wontfix
status-firefox-esr52: --- → affected
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
Summary: On playing particular ISO-BMFF stream with Media Source Extension, playback gets stuck. → When stream ID change during internal seeking, waiting promise will never be resolved.
Do I have permission to use this content to write a regression test?
Flags: needinfo?(sausharm)
Yes, you can use it.
But we have to pull off the content from the server so can you please give us a sense of time we can work on keeping it only till then. After that we will remove it from server.
Flags: needinfo?(sausharm)
interestingly, I can only reproduce it with that particular audio track and on windows.

FFmpeg gives warnings about it... I'm guessing this causes a particular behaviour with the AAC MFT which trigger the broken code.
Comment on attachment 8844628 [details]
Bug 1342913: P2. Terminate draining operations when possible.

https://reviewboard.mozilla.org/r/117988/#review119770
Attachment #8844628 - Flags: review?(gsquelart) → review+
Could you please test this build on windows?
https://archive.mozilla.org/pub/firefox/try-builds/jyavenard@mozilla.com-eac639221065855a3750cb4f7d327e0ede376a5c/try-win32/firefox-55.0a1.en-US.win32.installer.exe

Still getting a time out on mac with the test... only on optimised build which is a pain...
Flags: needinfo?(sausharm)
I tested it on windows 10 and it is working fine.
Flags: needinfo?(sausharm)
May I know when this fix will be released? Because https://wiki.mozilla.org/RapidRelease/Calendar is showing that firefox52 is released on 7th march. So, does firefox52 has this fix?
Flags: needinfo?(jyavenard)
Depends on: 1345756
(In reply to Saurabh Sharma from comment #20)
> May I know when this fix will be released? Because
> https://wiki.mozilla.org/RapidRelease/Calendar is showing that firefox52 is
> released on 7th march. So, does firefox52 has this fix?

It will not unfortunately. Or maybe a point release, but that's out of my hands to decide
Flags: needinfo?(jyavenard)
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7fc8016ace00
P1. Add mochitest. r=gerald
https://hg.mozilla.org/integration/autoland/rev/f770cf70a30e
P2. Terminate draining operations when possible. r=gerald
For the record (and as I had indicated on #developer), the only reason this test failed is because this commit landed before bug 1345756.

Unfortunate that it got backed out.
Flags: needinfo?(jyavenard)
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/22f89dff7a80
P1. Add mochitest. r=gerald
https://hg.mozilla.org/integration/autoland/rev/cdf4db6ebcaf
P2. Terminate draining operations when possible. r=gerald
https://hg.mozilla.org/mozilla-central/rev/22f89dff7a80
https://hg.mozilla.org/mozilla-central/rev/cdf4db6ebcaf
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Track 53+/54+ as video playback issue.

Hi :jya,
Since this also affects 53/54, do you think the patch is worth uplifting to 53/54?
tracking-firefox53: ? → +
tracking-firefox54: --- → +
Comment on attachment 8844628 [details]
Bug 1342913: P2. Terminate draining operations when possible.

Approval Request Comment
[Feature/Bug causing the regression]: Always been there I think
[User impact if declined]: Intermittent stalls in YouTube and various sites using MSE. We've had various reports of stalls on some machines, never been able to consistently reproduce the problem before. Hopefully, this is it.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: link provided in one of the comments, new mochitest written
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: We break a dead conditions if detected. Can't be worse than before.
[String changes made/needed]:
Attachment #8844628 - Flags: approval-mozilla-beta?
Attachment #8844628 - Flags: approval-mozilla-aurora?
Comment on attachment 8844628 [details]
Bug 1342913: P2. Terminate draining operations when possible.

Intermittent stalls in YouTube is a huge impact for users. Aurora54+ & Beta53+.
Attachment #8844628 - Flags: approval-mozilla-beta?
Attachment #8844628 - Flags: approval-mozilla-beta+
Attachment #8844628 - Flags: approval-mozilla-aurora?
Attachment #8844628 - Flags: approval-mozilla-aurora+
grafting 406228:af228c5bb2a6 "Bug 1342913: P2. Terminate draining operations when possible. r=gerald a=gchang"
merging dom/media/MediaFormatReader.cpp
merging dom/media/MediaFormatReader.h
warning: conflicts while merging dom/media/MediaFormatReader.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging dom/media/MediaFormatReader.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

seems this need rebasing for beta
Flags: needinfo?(jyavenard)
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/33aaa4a0dcc5bd9c10170b379abaf920bafff34a
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/69b1e67f3da3960f6c32df56d5e627c73c1b0a

Note that I had only applied for uplifting P2 (the code, not the mochitest). The mochitest will fail unless bug 1345756 is also uplifted
status-firefox53: affected → fixed
Flags: needinfo?(jyavenard)
Setting qe-verify- since it has automated coverage and it does not consistently reproduce.
Flags: qe-verify-
Is this something you wanted to consider nominating for ESR52 still or should we wontfix it?
Flags: needinfo?(jyavenard)
This is not a security issue.
It's a usability issue. It will make playback such as YouTube stall intermittently.

So it would be nice to have it in 52, however I'm still puzzled on what the policies are in regards to ESR uplifts.
Flags: needinfo?(jyavenard)
Bug fixes for non-security/stability issues are within scope for ESR if the issue is severe enough. Personally, I think random stalls during playback on Youtube are enough to at least warrant that discussion happening :)
See Also: → 1345898
The rebased Beta patch for Part 2 grafts cleanly to ESR52. I think we should at least nominate it for consideration given that breaking Youtube is kind of a big deal :P
Flags: needinfo?(jyavenard)
no problem from me here...
Flags: needinfo?(jyavenard)
Comment on attachment 8844628 [details]
Bug 1342913: P2. Terminate draining operations when possible.

Per comment 36 and subsequent discussion. Playback issues on Youtube seem like a severe enough issue to justify consideration IMO.
Attachment #8844628 - Flags: approval-mozilla-esr52?
Comment on attachment 8844628 [details]
Bug 1342913: P2. Terminate draining operations when possible.

Playback issues on Youtube have impact on users. Let's uplift this to ESR52.3.
Attachment #8844628 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: