Introduce the playback time into the MSE garbage collection.

Introduce the playback time into the MSE garbage collection.

The MSE spec - section 2.4.4 SourceBuffer Monitoring - mentions
what should be the media element state based on the playback time
and the MSE source buffer ranges.
There is currently no mechanism in the garbage collection to prevent
the current playback time from being garbage collected.
This CL protects the buffers corresponding to the current playback time.

BUG=None

[damienv1, 2014-06-19 15:37:42]

This CL is not complete (the current playback time is not passed to the
SourceBufferStream).

I'd like first to have your feedback on this CL and whether you consider it as
reasonable change.

[acolwell GONE FROM CHROMIUM, 2014-06-19 15:46:49]

On 2014/06/19 15:37:42, damienv1 wrote:
> This CL is not complete (the current playback time is not passed to the
> SourceBufferStream).
> 
> I'd like first to have your feedback on this CL and whether you consider it as
> reasonable change.

I don't think SourceBufferStream should be caching the current time. If we
really need this, I think it should be passed in via Append() and not cached.

[acolwell GONE FROM CHROMIUM, 2014-06-19 16:00:26]

https://codereview.chromium.org/341083004/diff/1/media/filters/source_buffer_...
File media/filters/source_buffer_stream.cc (right):

https://codereview.chromium.org/341083004/diff/1/media/filters/source_buffer_...
media/filters/source_buffer_stream.cc:904: }
I think you need a LastGOP equivalent so that the GOP doesn't get removed from
the backwards direction either. FirstGOPContainsNextBufferPosition() and
LastGOPContainsNextBufferPosition() should probably be renamed with
s/NextBufferPosition/CurrentPlaybackPosition/ and handle the time & position
based checks since both represent the "current position". Another way to think
if this is to essentially protect the time range between the current media time
and the timestamp on the "next buffer position". If a GOP intersects with that
range, then it shouldn't be removed.

https://codereview.chromium.org/341083004/diff/1/media/filters/source_buffer_...
media/filters/source_buffer_stream.cc:913:
!current_range->FirstGOPEarlierThanMediaTime(current_media_time_)) {
Don't you really want FirstGOPContainsMediaTime(current_media_time) ?

[damienv1, 2014-06-19 16:36:58]

Aaron, you don't want to protect just buffers located at the current playback
time but all buffers that come after (when garbage collecting from the front).
Otherwise, you might just garbage collect some buffers between the playback time
and the source buffer read position (when playback time advances, buffers won't
be there anymore if they have been garbage collected).

[damienv1, 2014-06-19 17:55:10]

Please take another look at the structure.
Once we agree on the structure, I will:
- update the unit tests
- introduce a callback so that ChunkDemuxer can get the playback time

[acolwell GONE FROM CHROMIUM, 2014-06-23 17:02:27]

On 2014/06/19 17:55:10, damienv1 wrote:
> Please take another look at the structure.
> Once we agree on the structure, I will:
> - update the unit tests
> - introduce a callback so that ChunkDemuxer can get the playback time

This seems overly invasive. ISTM we should be able to call
GarbageCollectIfNeeded() directly from SourceState::Append() after the
stream_parser_->Parse() call returns instead of inside
SourceBufferStream::Append(). It seems like that would result in a lot fewer
changes and get us a little closer the behavior outline in the spec. It also
might result in a perf win since we'd be running the GC algorithm only once per
append.

[damienv1, 2014-07-02 22:58:58]

Agree, the garbage collection could be invoked outside of the Append loop.

In that case, this CL needs to be broken:
1- move the garbage collection outside of the append loop
2- introduce the playback time into the MSE garbage collection.

[acolwell GONE FROM CHROMIUM, 2014-07-03 00:57:17]

On 2014/07/02 22:58:58, damienv1 wrote:
> Agree, the garbage collection could be invoked outside of the Append loop.
> 
> In that case, this CL needs to be broken:
> 1- move the garbage collection outside of the append loop
> 2- introduce the playback time into the MSE garbage collection.

Sounds like a good plan to me.

[acolwell GONE FROM CHROMIUM, 2014-09-05 23:59:00]

acolwell@chromium.org changed reviewers:
- acolwell@chromium.org

[servolk, 2015-10-09 22:14:24]

On 2014/07/03 00:57:17, acolwell GONE FROM CHROMIUM wrote:
> On 2014/07/02 22:58:58, damienv1 wrote:
> > Agree, the garbage collection could be invoked outside of the Append loop.
> > 
> > In that case, this CL needs to be broken:
> > 1- move the garbage collection outside of the append loop
> > 2- introduce the playback time into the MSE garbage collection.
> 
> Sounds like a good plan to me.

Since https://codereview.chromium.org/1008463002/ has landed, this is now
obsolete, so closing it.