Closed Bug 1249904 Opened 4 years ago Closed 4 years ago

Audio playback doesn't start in google drive

Categories

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

31 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
firefox44 --- affected
firefox45 --- affected
firefox46 --- verified
firefox47 --- verified

People

(Reporter: fred_spamfighter-mozaddon, Assigned: jya)

References

Details

(Keywords: regression)

Attachments

(2 files)

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
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
umm
Last work Firefox41, Broken since Firefox42

Regression window
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d4b6f85c0d42&tochange=e192ba2bd045
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jyavenard)
Yes, I missed the 2nd regression.
Blocks: 1185972
surprising, bug 1185972 should have only affected MSE.

will check.
Assignee: nobody → jyavenard
Flags: needinfo?(jyavenard)
Priority: -- → P2
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
No longer blocks: 1185972
central build as of time where 1185972 was committed I should add (hg commit e192ba2bd045)
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.
Flags: needinfo?(alice0775)
When you state that it worked with an old version of Firefox, could you state which version last worked for you?

thank you
Flags: needinfo?(fred_spamfighter-mozaddon)
(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
Flags: needinfo?(alice0775)
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.
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.
(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.
Flags: needinfo?(fred_spamfighter-mozaddon)
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
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 !
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()
Flags: needinfo?(jwwang)
Priority: P2 → P1
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...
Flags: needinfo?(ydelendik)
Checking...
Flags: needinfo?(jwwang)
Attached image 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.
I did notice that the content was different depending on the user agent sent... that sucks
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 ?
Flags: needinfo?(ydelendik) → needinfo?(jyavenard)
(In reply to Yury Delendik (:yury) from comment #21)
> The adobe range 

The above range
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
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).
Flags: needinfo?(jyavenard)
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.
(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?
(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.
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?
Flags: needinfo?(jwwang)
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.
Flags: needinfo?(jwwang)
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?
Flags: needinfo?(cpeterson)
OK. I will follow up with our Google contacts.
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/
Attachment #8723216 - Flags: review?(roc)
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.
Attachment #8723216 - Flags: review?(roc)
Blocks: 1251044
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
Attachment #8723216 - Flags: review+
Jean-Yves has contacted Google.
Flags: needinfo?(cpeterson)
https://hg.mozilla.org/mozilla-central/rev/6ecde9c54b08
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
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
Attachment #8723216 - Flags: approval-mozilla-aurora?
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
Attachment #8723216 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This would be good to verify on 46.
Flags: qe-verify+
Reproduced the issue on FX 41, Win 7.
Verified fixed FX 46b1, 47.0a2 (2016-03-14).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.