1512611 - HTMLMediaElement's buffered attribute not up to date after finishing fetching media on hypem.com

Status: NEW

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

Description

[Oana Botisan, Desktop QA]

Attachment: not buffered.mov

[Affected versions]:
- Nightly 65.0a1
- DevEd 65.0b2
- Firefox 63.0.3

[Affected platforms]:
- Windows 10 x64
- Windows 7 x32
- Ubuntu 18.04
- macOS 10.14

[Steps to reproduce]:
1. Go to: https://hypem.com/artist/filous
2. Play any of the songs.
3. Look at the player that's at the bottom of the page.

[Expected result]:
- The whole song is buffered.

[Actual result]:
- The last portion of the song is not buffered. 

[Regression range]:
- This is a regression. I can't reproduce the issue on Nightly from 2016-01-02. I will try to find a pushlog as soon as possible.

[Additional notes]:
- Please look at the attached video.
- The song still plays in that portion, but if you click on the progress bar nothing happens.
- If the page is refreshed a few times (5-6 times) the problem disappears.

Comment 1

[Oana Botisan, Desktop QA]

[Regression range]:

- last good build: 2016-08-04 (20160804030441)
- first bad build: 2016-08-05 (20160805030444)
- pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1576e7bc1bec7232e9e4ba78cce62526b1a6380b&tochange=d320ef56876f52db9bc0eb79554c7332d4793769

Comment 2

[Andreas Pehrson [:pehrsons]]

They are reading this from the `buffered` attribute in the event handler of the "progress" and "suspend" events.

The spec says this:
> Once the entire media resource has been fetched (but potentially before any of it has been decoded)
> 
>     Fire an event named progress at the media element.
> 
>     Set the networkState to NETWORK_IDLE and fire an event named suspend at the media element.


I see this in the log:
> [Child 31511: Main Thread]: D/nsMediaElementEvents 0x103f3c000 Dispatching event progress
> [Child 31511: Main Thread]: D/nsMediaElement 0x103f3c000 Buffered range 0 [0.0,2.0]
> [Child 31511: Main Thread]: D/nsMediaElementEvents 0x103f3c000 Dispatching event suspend
> [Child 31511: Main Thread]: D/nsMediaElement 0x103f3c000 Buffered range 0 [0.0,2.0]

It seems to me that when we're done fetching the data, it should be part of the `buffered` range.

Looking at code it seems that the "progress" and "suspend" events stem from [1], while the `buffered` range is set by [2]. I wonder if the `buffered` range is not updated on the last "progress" because, even though we've finished fetching the resource, we haven't demuxed it yet.
I also wonder whether the intention of the spec is that we should have the guarantee that `buffered` is updated before "progress". I don't see much use of `buffered` if we don't. One would have to resort to polling it then.

jya, do you know what we should expect here?


[1] https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/dom/media/ChannelMediaResource.cpp#407
[2] https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/dom/media/MediaFormatReader.cpp#3043

Comment 3

[Jean-Yves Avenard [:jya]]

I don't think the two readyState and networkState are related.

It's even more obvious with MSE, where the buffered range is changed during the coding frame processing algorithm, while the network state is done on the main thread after the appendBuffer call.

The spec also clearly states that the buffered range is "at the time the attribute is evaluated. Users agents must accurately determine the ranges available, even for media streams where this can only be determined by tedious inspection" you can't do that synchronously between fetching the data and parsing it.

Unless we delay the notification and change to networkState only once the MFR has finished parsing the content

Comment 4

[Andreas Pehrson [:pehrsons]]

Perhaps that's what we need to do.

I'm marking this a P3 because the spec is not explicit about this. It does however work in chrome so for webcompat it still makes sense to adapt a bit here.

Comment 5

[Oana Botisan, Desktop QA]

I can reproduce this issue using 60.5.2esr on https://www.shutterstock.com/music/genre/Rock.

Comment 6

[Alin Ilea, Desktop Test Engineering]

Reproducible on Mac OS 10.12 in latest Nightly 69.0a1 (2019-06-10)
Build ID 20190610214846
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:69.0) Gecko/20100101 Firefox/69.0

Comment 7

[Patricia Lawless]

Hi Nils, do you think this can be looked at for 70?

Comment 8

[Nils Ohlmeier [:drno]]

From the pushlog bug 1274626 appears to be the most likely to cause problems in this area.

But I'm not convinced that we really understand the proper the solution here.

jya: is what you described in comment #3 really the solution we should pursue here?
Do we know if this is what Chrome does?

Comment 9

[Augusto Pace]

Hi,

I'm able to reproduce this issue using Windows with Firefox version NIghtly 75.0a1 (2020-02-20). Marking that flag as affected.

Comment 10

[Jean-Yves Avenard [:jya]]

(In reply to Nils Ohlmeier [:drno] from comment #8)

From the pushlog bug 1274626 appears to be the most likely to cause problems in this area.

But I'm not convinced that we really understand the proper the solution here.

jya: is what you described in comment #3 really the solution we should pursue here?
Do we know if this is what Chrome does?

Clearing my queue.

My earlier comment is a way to change the observable behaviour while still be spec compliant. But we are spec compliant today.

To me it seems to be a site issue, they read the buffered range once, the code could be listening to timechange instead and update the graph as needed.