Don't immediately fire canplaythrough event when mp4 has no duration

RESOLVED FIXED in Firefox 57

Status

()

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox57+ fixed, firefox58 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
see bug 1404278 comment 9.

If the video has no duration, it is read as infinite.

We will fire canplaythrough immediately even though we may not have enough data.
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8917001 [details]
Bug 1407243 - Don't immediately fire canplaythrough for infinite/live streams.

https://reviewboard.mozilla.org/r/188018/#review193478
Attachment #8917001 - Flags: review?(jwwang) → review+

Comment 3

a year ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f2fa0f0d781
Don't immediately fire canplaythrough for infinite/live streams. r=jwwang
(Assignee)

Comment 4

a year ago
[Tracking Requested - why for this release]: has impact on videos starting to early and causing stuttering audio.
(Assignee)

Updated

a year ago
tracking-firefox57: --- → ?
Would it be worth adding some test case?
Is this a regression?
(Assignee)

Comment 7

a year ago
(In reply to Julien Cristau [:jcristau] from comment #6)
> Is this a regression?

hard to say.. we just sucked on this one (part of the spec)
Priority: -- → P3
https://hg.mozilla.org/mozilla-central/rev/6f2fa0f0d781
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assuming that MP4 files with no duration info is uncommon and therefore this issue may not be so widespread to need a fix in 57. Recommend letting the fix ride the 58 train.
status-firefox57: --- → wontfix
tracking-firefox57: ? → -
(Assignee)

Comment 10

a year ago
(In reply to Ritu Kothari (:ritu) from comment #9)
> Assuming that MP4 files with no duration info is uncommon and therefore this
> issue may not be so widespread to need a fix in 57. Recommend letting the
> fix ride the 58 train.

:ritu on the contrary...

MP4 file with no duration is very common, it's how all live streams are made.

so we end up stuttering playing live streams on startup.

Please reconsider...
Flags: needinfo?(rkothari)
(Assignee)

Updated

a year ago
status-firefox57: wontfix → affected
tracking-firefox57: - → ?
Thanks Jean-Yves. I was waiting for someone to tell me how wrong my assumption was. You can thank me for giving you the pleasure of doing so. :) Please nominate the patch for uplift to 57.
tracking-firefox57: ? → +
Flags: needinfo?(rkothari)
(Assignee)

Comment 12

a year ago
Comment on attachment 8917001 [details]
Bug 1407243 - Don't immediately fire canplaythrough for infinite/live streams.

Approval Request Comment
[Feature/Bug causing the regression]: unsure
[User impact if declined]: Playback starts when we can't guarantee smooth and continuous playback. This causes audio to stutter
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: https://tjenkinson.github.io/firefox-mse-audio-glitch/, ensure the canplaythrough event isn't fired (only 5s worth of data is added on this site)
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: low
[Why is the change risky/not risky?]: some sites may incorrectly assumed the canplaythrough was fired very early before adding new data. such canplaythrough event won't be fired until more data is added. However, this would only point at non compliant sites.
[String changes made/needed]: none
Attachment #8917001 - Flags: approval-mozilla-beta?
Comment on attachment 8917001 [details]
Bug 1407243 - Don't immediately fire canplaythrough for infinite/live streams.

Distorted/stuttering audio is not good, Beta57+
Attachment #8917001 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 14

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/e54dfb628cfa
status-firefox57: affected → fixed
Flags: qe-verify+

Comment 15

a year ago
I tested this bug using an older version of Nightly (2017-10-10) on Windows 10x64 and Ubuntu 16.04 x64 using the steps from comment 12. The result on the old version of Nightly is the same as on the latest Nightly, the stuttering audio is played continuously. 

On Mac OS X 10.11, I got another result. About 2 seconds in the audio there was a glitch in the stuttering audio. This result I got both on Nightly from 2017-10-10 and latest Nightly.

Can you please give me more information about what exactly should happen when I run this steps? And on which platform should I test on?

Thank you.
Flags: needinfo?(jyavenard)
(Assignee)

Comment 16

a year ago
the page has been changed to no longer rely on autoplay... 

looking at this page:
https://jyavenard.github.io/htmltests/tests/mse_mp4/bipbop.html

without the page, you should see the canplaythrough event being fired even though we have less than 10s of content
with the fix, it shouldn't fire.

However, this page:
https://jyavenard.github.io/htmltests/tests/mse_mp4/bipbop.html?eos=1&eosat=-1&duration=-1&init=0

will show that canplaythrough will be fired even if less than 10s of data has been buffered, because the download is marked as completed (endOfStream was called)
Flags: needinfo?(jyavenard)
(Assignee)

Comment 17

a year ago
ah sorry, this video does have a duration set, so the problem won't occur.

Comment 18

a year ago
I talked to :jya and  did everything he suggested, but I couldn't reproduce the bug.
Does anyone have another idea of how I could reproduce it? 
Thank you.

Comment 19

a year ago
Like I mentioned in previous comments, I tried to reproduce the bug, but i couldn't. The links I tried were the ones mentioned in the above comments and another method that :jya suggested:

- get the code of that  https://tjenkinson.github.io/firefox-mse-audio-glitch/ (available in a git repository there: https://github.com/tjenkinson/firefox-mse-audio-glitch/)  and remove that line https://github.com/tjenkinson/firefox-mse-audio-glitch/blob/master/script.js#L15
- modify the index.html to make the audio element autoplay in  https://github.com/tjenkinson/firefox-mse-audio-glitch/blob/master/index.html#L5
- drag the resulted .html in the browser.
But even with this method I didn't managed to reproduce the bug on an older version of Nightly (2017-10-10).

I will take out the qe-verify+ flag, until we can get a valid method that might helps us reproduce the bug.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.