Closed Bug 1538023 Opened 1 year ago Closed 11 months ago

MediaCache sometimes evicts data at the current playback position and doesn't try to download it again

Categories

(Core :: Audio/Video: Playback, enhancement, P2)

Unspecified
Android
enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

Details

(Whiteboard: [gvtv:p2] [media-q2] [media-triaged])

Attachments

(2 files, 3 obsolete files)

I've been testing 4K60FPS video on the FireStick4K using src=url video, and discovered that often the MediaCache evicts data at the current playback position. This coincides with a playback stall, so I assume this is causing the stall.

Note: the FireStick is Android, and we use a media cache size of 32MB there, so the problem is exacerbated there. The same problem would exist on desktop, but we've likely not hit it since the MediaCache is big enough there that it's not stressed.

Assignee: nobody → cpearce
OS: Unspecified → Android
Whiteboard: [gvtv] → [gvtv:p1]
Rank: 10
Priority: -- → P2
Whiteboard: [gvtv:p1] → [gvtv:p1] [media-q2]
Whiteboard: [gvtv:p1] [media-q2] → [gvtv:p1] [media-q2] [media-triaged]

[gvtv:p2] because MediaCache is only used for static video files and YouTube uses MSE.

Whiteboard: [gvtv:p1] [media-q2] [media-triaged] → [gvtv:p2] [media-q2] [media-triaged]

Chris - is there a possibility that a user could run into this issue on (an adult) site that doesn't use MSE?

Flags: needinfo?(cpearce)

(In reply to Adam Stevenson [:adamopenweb] from comment #2)

Chris - is there a possibility that a user could run into this issue on (an adult) site that doesn't use MSE?

Yes, but to repro this a user needs a large video, a fast internet connection, and a small media cache. Once bug 1540573 lands, I expect the intersection of instances of the "small media cache" problem and the "large video" problem will go down dramatically, and so this is likely to not be a problem.

I've not observed this failure with 720p video, at least on the Stick4K.

Flags: needinfo?(cpearce)

So the issue is the MediaCache is evicting data that the MediaDecoderStateMachine uses to trigger entering a buffering state, and if this coincides with the MDSM's decoded sample queues being full, there's nothing to cause us to re-download that evicted data, and we end up getting indefinitely stuck in a buffering state.

What happens is our audio and video demuxers advance with different read cursors; the audio runs ahead 2 seconds. The MediaCache tries to keep a record of whether a "block" in the cache is a "read ahead" block, or a "played" block. The MediaCache records the offset at which each MediaCacheStream was last read in MediaCacheStream::mStreamOffset, and assumes blocks before that offset are "played" and can be evicted.

The MDSM assumes that it needs to start buffering if the lesser of the end of the decoded audio and end of the decoded video data isn't in our buffered ranges. https://searchfox.org/mozilla-central/rev/f0ef51bfafc8ad0c3a2f523bf076edc57dc4891a/dom/media/MediaDecoderStateMachine.cpp#3282

In practice the lesser will be the video demuxer's read cursor. However when the MediaCache is full it will evict data before the last read on the stream. If the last read on the stream was a read from the audio demuxer, this will end up being the greater of the end of the decoded audio and end of the decoded video data, as we run the audio decoder about 2 seconds ahead.

Thus the MediaCache will evict the data which causes the MDSM to go into buffering mode, and also the data the MDSM would need to get out of buffering mode.

If the MDSM's happens to have full decoded sample queues, or if all data required to fill the MDSM's sample queues are cached in MediaResourceIndex's, the MediaResource won't be read from, and so the MediaCache won't update and re-download the data around the current playback position, and so we won't ever get out of buffering state, and playback will stall indefinitely.

Re-downloading the evicted data would be bad, we're better off not evicting it in the first place.

I think the solution here is to tweak MediaCache::PredictNextUse such that it assumes the read offset is the lesser of the read cursors of the MediaResourceIndex's as well as MediaCacheStream::mStreamOffset. Then we'll have the same assumption as the MDSM's logic. Then we won't evict data which the MDSM's buffering logic uses to enter/exit buffering state.

In order for the MediaCache to know what the last read offset of each
MediaResource's MediaResourceIndex was, we need to keep track of each
MediaResource's Index's.

This needs to be thread safe, as we'll be writing to the MediaResource's list
of Indexes on random demuxer threads, and reading from it on the MediaCache
thread.

This ensures the MediaCache can take into account the last reads by the
demuxers which use MediaResourceIndexes.

The last read offset on the MediaResourceIndex needs to be atomic, as it's
written on demuxer threads, and will be read on the MediaCache thread. We don't
want to lock while accessing the read offset, as that could cause lock
contention, which is one of the things that MediaResourceIndex was designed to
avoid.

Depends on D29788

Attachment #9062406 - Attachment is obsolete: true
Attachment #9062407 - Attachment is obsolete: true
Attachment #9062408 - Attachment is obsolete: true

TimeUnits with a negative infinity value are used in the next patch.

When under pressure, the MediaCache evicts data before the last read on a
stream. We typically have two demuxers reading from different offsets in a
stream. So if the MediaCache is under pressure, it may end up evicting data
between the two demuxers.

The MediaDecoderStateMachine currently goes into buffering state if there's
insufficient data available beginning at the start of its queue of decoded
samples. However since the MediaCache evicts data behind the streams read
cursor, the data after the beginning of the sample queue may have already been
evicted by the media cache. This will cause the MediaDecoderStateMachine to
enter a buffering state, and if its sample queues are full, there will be no
demuxers reading to cause the MediaCache to download the data between the two
demuxers, and we'll get stuck in buffering state indefinitely.

So change the MediaDecoderStateMachine to instead check whether there's
insufficient data available at the end of the decoded sample queues. This means
we won't get stuck in buffering state. Note the MediaCache may still evict data
which the other demuxer needed, causing us to re-request it, but at least we
won't get stuck in buffering state.

Depends on D30309

Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e38a89a6d65
Add support for -Inf to media::TimeUnits. r=jya
https://hg.mozilla.org/integration/autoland/rev/98918b9e369c
Change MDSM::HasLowBufferedData() to consider data buffered after end of decoded data rather than start. r=jya
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.