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

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

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 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1407243 - Don't immediately fire canplaythrough for infinite/live streams.

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

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

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

https://reviewboard.mozilla.org/r/188018/#review193478

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f2fa0f0d781
Don't immediately fire canplaythrough for infinite/live streams. r=jwwang

Comment 4

[Jean-Yves Avenard [:jya]]

[Tracking Requested - why for this release]: has impact on videos starting to early and causing stuttering audio.

Comment 5

[Blake Wu [:bwu][:blakewu]]

Would it be worth adding some test case?

Comment 6

[Julien Cristau [:jcristau] (back April 22)]

Is this a regression?

Comment 7

[Jean-Yves Avenard [:jya]]

(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)

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6f2fa0f0d781

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 10

[Jean-Yves Avenard [:jya]]

(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...

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 12

[Jean-Yves Avenard [:jya]]

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

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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

Distorted/stuttering audio is not good, Beta57+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Oana Botisan, Desktop QA]

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.

Comment 16

[Jean-Yves Avenard [:jya]]

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)

Comment 17

[Jean-Yves Avenard [:jya]]

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

Comment 18

[Oana Botisan, Desktop QA]

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

[Oana Botisan, Desktop QA]

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.