976037 - Implement eviction algorithm for MediaSourceExtensions

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[owpard]

The media source extensions implementation currently does not evict any data. All data is held in memory and the method for appending new data results in copying and moving memory.

The Media Source Extensions spec lists a coded frame eviction algorithm. Something like this should be implemented.

Comment 1

[owpard]

Attachment: Implment an eviction algorithm

This implements an eviction algorithm. It modifies the existing system so that a SourceBufferResource has a queue containing the data that is appended to it. 

The queue holds instances of ResourceItem which is an array of the bytes. Appending data to the SourceBufferResource pushes this onto the queue. As items are played from the queue. Data is evicted once it reaches a size threshold. This pops the items off the front of the queue and deletes it. If an eviction happens then the MediaSource that effectively owns the SourceBufferResource is notified (done in SourceBuffer::AppendData) which then requests all SourceBuffers to evict data up to approximately the same timepoint.

The timestamps used to evict are estimated using a rough guess. Hopefully improvements to this can be done in a further patch if needed.

There is currently no 'buffer full' flag maintained. It's assumed that data can be added always. Eviction does not remove any data before the current playback point. Since data is kept in memory we should refine in a future patch (or this one if the reviewer wants) more stringent buffer management rules. I'm unsure what good rules are here.

The demos on the following page work with this patch: http://cd.pn/mse/

I tested 'basic demo', 'dash player' and 'youtube player' with the 'feelings2' mpd. I was unable to test the youtube player with multiple streams as it requires being able to reset the stream which is not implemented.

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8380605 [details] [diff] [review]
Implment an eviction algorithm

Review of attachment 8380605 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

::: content/media/mediasource/MediaSource.h
@@ +84,5 @@
>    {
>      return mDecoder;
>    }
>  
> +  // Called by SourceBuffer's to notify this MediaSource that data has

No apostrophe.

::: content/media/mediasource/MediaSourceDecoder.h
@@ +78,5 @@
>    {
>      return mAudioReader;
>    }
>  
> +  double GetDuration();

Move this up to below GetSeekable and add MOZ_OVERRIDE.

::: content/media/mediasource/SourceBuffer.cpp
@@ +98,5 @@
>  
> +int64_t
> +SubBufferDecoder::ConvertToByteOffset(double aTime)
> +{
> +  // Uses a conversion based on (aTime/duration) * Length.  For the

Lower case l in length.

@@ +106,5 @@
> +  double duration = mParentDecoder->GetDuration();
> +  if (duration <= 0.0 || IsNaN(duration)) {
> +    return -1;
> +  }
> +  int64_t length = GetResource()->GetLength();

MOZ_ASSERT length is > 0

@@ +382,5 @@
> +    double s = ranges->Start(i, rv);
> +    double e = ranges->End(i, rv);
> +    startTime = (startTime < 0.0) ? s : (startTime < s ? startTime : s);
> +    endTime = endTime > e ? endTime : e;
> +  }

You could call ranges->Normalize(), then use ranges->Start(0) and ranges->End(ranges->Length() - 1) for start and end times.

::: content/media/mediasource/SourceBufferList.cpp
@@ +103,5 @@
>    }
>  }
>  
>  void
> +SourceBufferList::Evict(double aStart, double aEnd) {

Brace on new line

Comment 3

[owpard]

Attachment: Implement and eviction algorithm

Updates review comments ready for landing.

Comment 4

[owpard]

Attachment: Implement an eviction algorithm

Minor changes to include a couple of header files to fix non-unified builds picked up by try. r+ carried forward.

Comment 5

[owpard]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d8a39b9c13e

Comment 6

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

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/320198fc474f for test failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=35309541&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=35309079&tree=Mozilla-Inbound

Comment 7

[owpard]

Attachment: Implement an eviction algorithm

There was a test failure in the original patch. This fixes that. I'll attach an interdiff shortly.

Comment 8

[owpard]

Attachment: Interdiff between original patch and fix for test breakage

This is the interdiff between attachment 8383398 [details] [diff] [review] and the original patch that was backed out. The issue was that 'GetDuration' on the MediaSourceDecoder is expected to return the duration as computed by the Decoder, whereas the internal calculations for eviction need to use the MediaSource view of the duration.

Comment 9

[owpard]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b295844b85b

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9b295844b85b