1249904 - Audio playback doesn't start in google drive

Status: VERIFIED FIXED

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

Description

[fremea]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160210153822

Steps to reproduce:

On an clean profile just open a google drive link to an audio file such as https://drive.google.com/open?id=0B6fsosFSSxYZYWR3X1VxVWd5QzA


Click on the start playback button.



Actual results:

Button is triggered but playback or downloading audio buffer doesn't start.


Expected results:

It works as expected in IE or chrome based browser or old firefox version.

The only workaround is to manually start playback with 

document.querySelector("audio").play(); 

in web console. This is not acceptable for average users.

Here are some references to this issue:

https://support.mozilla.org/fr/questions/1027929

https://www.youtube.com/watch?v=FUJki848WFI

Comment 1

[Loic]

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c148f0b0c8b4&tochange=72c2f9ebf4bc

Comment 2

[Alice0775 White]

umm
Last work Firefox41, Broken since Firefox42

Regression window
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d4b6f85c0d42&tochange=e192ba2bd045

Comment 3

[Loic]

Yes, I missed the 2nd regression.

Comment 4

[Jean-Yves Avenard [:jya]]

surprising, bug 1185972 should have only affected MSE.

will check.

Comment 5

[Jean-Yves Avenard [:jya]]

I have reverted to a central build and reverted all changes introduced by bug 1185972 and playback still doesn't start.

so it's definitely not 1185972

Comment 6

[Jean-Yves Avenard [:jya]]

central build as of time where 1185972 was committed I should add (hg commit e192ba2bd045)

Comment 7

[Jean-Yves Avenard [:jya]]

I have now reverted to the revision right before bug 1185886 was applied.

it still doesn't start.

So the regression range appears incorrect.

Alice0775, are you certain of this regression range?

Seems to me the issue is in the google player itself; changing the user agent shows that it behaves differently between chrome and safari.

Comment 8

[Jean-Yves Avenard [:jya]]

When you state that it worked with an old version of Firefox, could you state which version last worked for you?

thank you

Comment 9

[Alice0775 White]

(In reply to Jean-Yves Avenard [:jya] from comment #7)
> I have now reverted to the revision right before bug 1185886 was applied.
> 
> it still doesn't start.
> 
> So the regression range appears incorrect.
> 
> Alice0775, are you certain of this regression range?
> 
> Seems to me the issue is in the google player itself; changing the user
> agent shows that it behaves differently between chrome and safari.

Tried with clean profile again, And I got same range.
Good:
http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1437969468/

Bad:
http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1437972943/

And via local build,
Last Good: 6681786341a9
Bad : e192ba2bd045

Comment 10

[Jean-Yves Avenard [:jya]]

well, when I do a checkout of d4b6f85c0d4 which is parent of http://hg.mozilla.org/mozilla-central/rev/91de55bfe09a03 (itself an ancestor of http://hg.mozilla.org/mozilla-central/rev/e192ba2bd045)

playback will *not* start.

So I don't know what's going on, but the regression range you provided isn't pointed to what caused this regression.

But having said that, in the range provide, it includes a backout. so it may be a false positive.

Comment 11

[Jean-Yves Avenard [:jya]]

I should add that I see no error on our end ; when pressing "play" the content is loaded. But the play() function isn't called.

Everything indicates an problem in the JS.

Not sure what event they are waiting for that somehow we wouldn't be firing, but i have my doubts that the issue is in the audio element code.

Comment 12

[fremea]

(In reply to Jean-Yves Avenard [:jya] from comment #8)
> When you state that it worked with an old version of Firefox, could you
> state which version last worked for you?
> 
> thank you

I'm sorry, I just reported this statement because I've read it in the reference links I gave and I remember it previously worked in ff but I can't remember the last working version.

Comment 13

[Jean-Yves Avenard [:jya]]

FWIW: I can reproduce the problem in build 41 (http://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-release-macosx64-debug/1445573068/)

the same media element with standard playback control works perfectly:
http://people.mozilla.org/~jyavenard/tests/drive.html

Comment 14

[Jean-Yves Avenard [:jya]]

That video is very sad :( https://www.youtube.com/watch?v=FUJki848WFI

because Firefox certainly hasn't dropped support for playing MP3; now if only Google's JS actually called play() it would help !

Comment 15

[Jean-Yves Avenard [:jya]]

JW, you're JS-foo is better than mine.
Could you have a look at the JS and sees why it's not calling play() when you're calling play()?

could it be similar to bug 1181055?

an onclick calling play()

Comment 16

[Jean-Yves Avenard [:jya]]

Yury could you check their JS and see what's going on there?

Or maybe we should just contact Google and get them to investigate...

Comment 17

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

Checking...

Comment 18

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

Attachment: bug1249904.png

The play/pause button has a 'click' listener on Chrome while I can't find it on Firefox. I guess there is something wrong to the DOM tree.

Comment 19

[Jean-Yves Avenard [:jya]]

I did notice that the content was different depending on the user agent sent... that sucks

Comment 20

[Yury Delendik (:yury)]

Jean-Yves, on Mac OSX is was fixed something in range https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e8cc99a3321cd07d081ac501dd0dbf4c4d933061&tochange=b493cde48ac0fefdb6808abf8ed3745f27f22c57 (bug 1229657?)

Comment 21

[Yury Delendik (:yury)]

The adobe range also this fixes playback on Windows. To be precise by https://hg.mozilla.org/integration/mozilla-inbound/rev/be3eab5c43f0 (FF45)

Shall we mark this bug as fixed by bug 1229256 ?

Comment 22

[Yury Delendik (:yury)]

(In reply to Yury Delendik (:yury) from comment #21)
> The adobe range 

The above range

Comment 23

[Alice0775 White]

I can still reproduce the problem on Nightly47.0a2, Aurora46.0a2 and beta45.0b8.

https://hg.mozilla.org/mozilla-central/rev/789a12291942763bc1e3a89f97e0b82dc1c9d00b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0 ID:20160223030304

https://hg.mozilla.org/releases/mozilla-aurora/rev/611418c0941827db745c285c03557339d73a53c2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20160223004110

https://hg.mozilla.org/releases/mozilla-beta/rev/73fd4c217368b38a64aa2e59de8e3fc5bef8eedb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0 ID:20160221141421

Comment 24

[Yury Delendik (:yury)]

Interesting opening it via folder view plays that in nightly (https://drive.google.com/open?id=0B_yLYgEUIJdKVzB1LWJ3bHg4Q2M), however providing direct link (as in comment #0) breaks it (https://drive.google.com/open?id=0B_yLYgEUIJdKVTh3UUFkak5mMmM).

Comment 25

[Yury Delendik (:yury)]

There is a difference between events:

- Chrome's audio element fires events on play button press "loadstart", "stalled", "durationchange", "loadedmetadata", "loadeddata", "canplay", "canplaythrough", and more ...
- Firefox's audio element "loadstart", "durationchange", "loadedmetadata", "loadeddata", "suspend".

We never get the "canplay" event with this media (reported in comment 0), there is a network request and it's aborted. If I press the play button and then simulate the "canplay" event, e.g.:

  var e = document.createEvent('Event'); e.initEvent('canplay', false, false);
  var a = document.querySelector('audio'); a.dispatchEvent(e);

the playback is started.

Comment 26

[Jean-Yves Avenard [:jya]]

(In reply to Yury Delendik (:yury) from comment #20)
> Jean-Yves, on Mac OSX is was fixed something in range
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=e8cc99a3321cd07d081ac501dd0dbf4c4d933061&tochange=b493
> cde48ac0fefdb6808abf8ed3745f27f22c57 (bug 1229657?)

it's not fixed in my nightly build.

(In reply to Yury Delendik (:yury) from comment #25)
> There is a difference between events:
> 
> - Chrome's audio element fires events on play button press "loadstart",
> "stalled", "durationchange", "loadedmetadata", "loadeddata", "canplay",
> "canplaythrough", and more ...
> - Firefox's audio element "loadstart", "durationchange", "loadedmetadata",
> "loadeddata", "suspend".
> 
> We never get the "canplay" event with this media (reported in comment 0),
> there is a network request and it's aborted. If I press the play button and
> then simulate the "canplay" event, e.g.:

what do they set as preload value for the audio element?
if it's metadata , we won't fire canplay/canplaythrough


> 
>   var e = document.createEvent('Event'); e.initEvent('canplay', false,
> false);
>   var a = document.querySelector('audio'); a.dispatchEvent(e);
> 
> the playback is started.

that's great feedback.. thanks...

any ideas why the network request is aborted? and who aborts it?

Comment 27

[Jean-Yves Avenard [:jya]]

(In reply to Yury Delendik (:yury) from comment #20)
> Jean-Yves, on Mac OSX is was fixed something in range
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=e8cc99a3321cd07d081ac501dd0dbf4c4d933061&tochange=b493
> cde48ac0fefdb6808abf8ed3745f27f22c57 (bug 1229657?)

ok, the preload is set to "metadata"
"Hints to the user agent that the author does not expect the user to need the media resource, but that fetching the resource metadata (dimensions, track list, duration, etc), and maybe even the first few frames, is reasonable. If the user agent precisely fetches no more than the metadata, then the media element will end up with its readyState attribute set to HAVE_METADATA; typically though, some frames will be obtained as well and it will probably be HAVE_CURRENT_DATA or HAVE_FUTURE_DATA. When the media resource is playing, hints to the user agent that bandwidth is to be considered scarce, e.g. suggesting throttling the download so that the media data is obtained at the slowest possible rate that still maintains consistent playback. "

so while we *could* have readyState == HAVE_FUTURE_DATA (which would cause the canplay event to be fired), there's no requirement to do so.

Prior 45, we would *never* have fired canplay if preload was set to metadata.
In 45 we now also check the buffered range to determine if canplay can be fired which is an improvement and got us closer to the spec above. We use a 250ms threshold however to determine if readyState can be HAVE_FUTURE_DATA.

We could lower the 250ms threshold that was added is too much of a delay for canplay.

Here however we have an issue that the buffered range is still empty, even after loadeddata is fired.

Comment 28

[Jean-Yves Avenard [:jya]]

We have an empty buffered range because the MediaResource doesn't have a length.

https://dxr.mozilla.org/mozilla-central/source/dom/media/VideoUtils.cpp#117
"  // If we can't determine the total size, pretend that we have nothing
  // buffered. This will put us in a state of eternally-low-on-undecoded-data
  // which is not great, but about the best we can do.
"

we can't determine the duration of the cached ranges if we don't have the global size. So in the case i was talking about, the override of using the buffered range to determine the readyState doesn't help

JW, why is it that preload doesn't have a default value of "auto" ? I though it was the default for media element?

Comment 29

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

https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp#1173

No, default preload is 'metadata' for desktop browsers when "media.preload.default" is not specified in the prefs.

Comment 30

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

Btw, preload defaults to 'none' on mobile platforms.

https://dxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#315
https://dxr.mozilla.org/mozilla-central/source/mobile/android/app/mobile.js#624
https://dxr.mozilla.org/mozilla-central/source/mobile/android/b2gdroid/app/b2gdroid.js#581

Comment 31

[Jean-Yves Avenard [:jya]]

I believe I've found the bug in our code that would cause the buffered range to be empty (which if it wasn't would cause canplay to fire).

Having said that, we should contact google to fix their player and make preload="auto", or better, don't listen on the canplay event at all, because on mobile platforms it won't be fired at all until play() is called.

so if google is waiting for canplay() to fire to call play() playback will never start there.

Chris, is this something you could report to them?

Comment 32

[Chris Peterson [:cpeterson]]

OK. I will follow up with our Google contacts.

Comment 33

[Jean-Yves Avenard [:jya]]

Attachment: MozReview Request: Bug 1249904: Use content-range values to determine length if content-length is unknown. r?roc

While currently unused, the functionality of mByteRange was preserved.
To avoid unforeseen regressions, we only error on an invalid Content-Range response if mByteRange was set, otherwise the error is ignored and code proceed as before.

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

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8723216 [details]
MozReview Request: Bug 1249904: Use content-range values to determine length if content-length is unknown. r?roc

https://reviewboard.mozilla.org/r/36413/#review32991

::: dom/media/MediaResource.cpp:247
(Diff revision 1)
> +      acceptsRanges = NS_SUCCEEDED(rv);

I'd call this gotRangeHeader. A buggy server might accept range requests but not send Content-Range.

::: dom/media/MediaResource.cpp:280
(Diff revision 1)
> +        contentLength = rangeTotal;

I think we should use the max of contentLength and rangeTotal. I know of no reason to ignore rangeTotal just because we have contentLength. We know contentLength can be incorrect.

Comment 35

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8723216 [details]
MozReview Request: Bug 1249904: Use content-range values to determine length if content-length is unknown. r?roc

https://reviewboard.mozilla.org/r/36413/#review33007

Comment 36

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ecde9c54b08

Comment 37

[Chris Peterson [:cpeterson]]

Jean-Yves has contacted Google.

Comment 38

[Carsten Book [:Tomcat]]

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

Comment 39

[Jean-Yves Avenard [:jya]]

Comment on attachment 8723216 [details]
MozReview Request: Bug 1249904: Use content-range values to determine length if content-length is unknown. r?roc

Approval Request Comment
[Feature/regressing bug #]: 1249904
[User impact if declined]: some JS players (in particular Google drive player) won't work
[Describe test coverage new/current, TreeHerder]: in central for a week
[Risks and why]: Low, patch was written with uplifting in mind so that no regressions could be introduced and that the change is only active for servers that are broken to start with
[String/UUID change made/needed]: none

Comment 40

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8723216 [details]
MozReview Request: Bug 1249904: Use content-range values to determine length if content-length is unknown. r?roc

Fix for regression for particular JS audio players, please uplift to aurora

Comment 41

[Liz Henry (:lizzard) (relman/hg->git project)]

This would be good to verify on 46.

Comment 42

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/7efe8a135e72

Comment 43

[Paul Silaghi, QA [:pauly]]

Reproduced the issue on FX 41, Win 7.
Verified fixed FX 46b1, 47.0a2 (2016-03-14).