New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 713423 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

MSE throws QuotaExceeded when a media track is disabled

Project Member Reported by servolk@chromium.org, Apr 19 2017

Issue description

When a media track is disabled, renderers stop reading from the corresponding DemuxerStream. MSE garbage collection algorithm currently tries to preserve the data that haven't been read from the stream yet, and that causes the SourceBufferStream for the disabled media track to fill up and start throwing QuotaExceeded exceptions.
 
One way to reproduce this is to start playing a very long YouTube video (longer than the MSE video buffer size, i.e. longer than let's say ~10-20 min, depending on video bitrate) and put the YouTube tab into background (with the background video track optimization turned on). After a while the video SourceBuffer will fill up and will start throwing QuotaExceeded exceptions.
Potential solutions are probably a combination of:
A) *if* the web app knows that there is huge amount of data preceding currentTime that will unlikely be needed soon, it can always issue an explicit remove() operation to evict that media. This is not a general fix though.

B) *if* the web app is continuing to buffer media for a currently disabled track (e.g., optimized video in a background tab in Chrome, assuming that feature is turned on), then perhaps it should stop fetching and appending that? (This could save network b/w at expense of video latency on resumption of foreground if nothing buffered for video at currentTime when that happens.) This also is not a general solution, since players would need to update their logic, and players that used muxed A/V stream sources would have difficulty (e.g. they might need to transmux to strip out the V from an A/V stream).

C) (servolk@ suggested something like this in discussion): have something like a null-decoder-sink for each disabled track (demuxerstream) -- this could get weird though: it seems a bit of a convoluted solution to simply make it appear to the GC heuristic that the stream reads have continued while it was disabled.

D) (my recommendation for a general fix): Patch the ChunkDemuxer GC heuristic to assume "pending_seek" is true when the stream is disabled. This knowledge of enabled/disabled is already plumbed into DemuxerStream and is used in the BG video optimization (experiment) and the web {audio,video}Tracks' API (also experimental) IIUC.

Cc: johnpallett@chromium.org
Labels: M-59
This will block launch of background track disable for MSE, so tagging as M59 and +johnpallett@ for FYI.
A)-B) are app specific, meaning the bg optimization would break sites that haven't been updated. Also, there's no clear signal for the page when the optimization is applied as 1) MediaTrack API is not shipped and b) even if it shipped, the optimization is currently not web exposed.

I'm wary of C) introducing Issue 709302 for MSE since IIUC we will discard the data needed to seek before the next available keyframe. Is it correct?

I assume with MediaTrack enabled and the site disabling the track itself, it's somewhat expected that the site should stop adding data to SourceBuffer, so we don't have to fix this use case?
Option C shouldn't have caused any additional issues, because MSE GC would remove data only up to the first key frame preceeding the current media playback position (i.e. the same position where we would try to seek the stream to when the track is re-enabled). But anyway, it seems like option C would add complexity, so we are considering other options now, which will hopefully be simpler.

Re your last question - the media tracks functionality is not marked stable yet, so it's inaccessible by default and sites are not using it. But even after sites start using the media tracks API to explicitly disable some tracks, we shouldn't expect them to necessarily stop appending data for disabled tracks. Some sites are using muxed a+v streams and they might still want to selectively disable one of the streams while continuing playing the other one, without remuxing their content. We should be able to handle this on the Chromium media pipeline level.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac9a37e939ced07b8d5f4f74f4829013895f0cf4

commit ac9a37e939ced07b8d5f4f74f4829013895f0cf4
Author: servolk <servolk@chromium.org>
Date: Thu Apr 20 21:23:00 2017

Fix MSE garbage collection for disabled media tracks

Renderers don't read from disabled demuxer streams, thus leaving
demuxer stream read position unchanged. But MSE garbage collection
currently stops removing data from the front of buffered ranges when
it reaches the last read position. This results in QuotaExceeded
exceptions being thrown when more data is appended to the disabled
stream. The fix is to seek the disabled stream before running MSE
GC, this will update the read position to the current media_time
and should allow the GC algorithm to remove data as expected.

BUG= 713423 

Review-Url: https://codereview.chromium.org/2827983004
Cr-Commit-Position: refs/heads/master@{#466136}

[modify] https://crrev.com/ac9a37e939ced07b8d5f4f74f4829013895f0cf4/media/filters/chunk_demuxer.cc
[modify] https://crrev.com/ac9a37e939ced07b8d5f4f74f4829013895f0cf4/media/filters/chunk_demuxer.h
[modify] https://crrev.com/ac9a37e939ced07b8d5f4f74f4829013895f0cf4/media/filters/source_buffer_state.cc
[modify] https://crrev.com/ac9a37e939ced07b8d5f4f74f4829013895f0cf4/media/filters/source_buffer_state.h
[modify] https://crrev.com/ac9a37e939ced07b8d5f4f74f4829013895f0cf4/media/test/pipeline_integration_test.cc

The fix has been merged, but keeping this bug open for now. We'll probably want to cherry-pick this fix into M59 branch after it bakes for a few days in the canary channel.
Labels: Merge-Request-59
This change has been in canary since 60.0.3077.0, and I've verified it with Mac Canary build 60.0.3080.0, let's merge it to M59.
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 25 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by sheriffbot@chromium.org, May 1 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Approved -Merge-Approved-59
Status: Fixed (was: Started)
The fix has been merged to M59 (see https://codereview.chromium.org/2843763003/), I'm not sure why this wasn't auto-updated by bugdroid. Marking as fixed manually. 
@#11, I've filed  bug 717630  to investigate why bugdroid didn't update this bug when https://codereview.chromium.org/2843763003/ merged into M59.

Sign in to add a comment