Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(167)

Issue 1349973002: Fix seeking back in the new MSE GC algorithm (Closed)

Created:
3 years, 6 months ago by servolk
Modified:
3 years, 6 months ago
Reviewers:
wolenetz
CC:
chromium-reviews, feature-media-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix seeking back in the new MSE GC algorithm BUG=532136

Patch Set 1 #

Total comments: 10

Patch Set 2 : CR feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+181 lines, -8 lines) Patch
M media/filters/chunk_demuxer_unittest.cc View 1 1 chunk +122 lines, -0 lines 0 comments Download
M media/filters/source_buffer_stream.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M media/filters/source_buffer_stream.cc View 1 5 chunks +58 lines, -8 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
wolenetz
Some nits, but as discussed in chat, this is likely to become the next patch ...
3 years, 6 months ago (2015-09-17 18:57:54 UTC) #2
servolk
https://codereview.chromium.org/1349973002/diff/1/media/filters/chunk_demuxer_unittest.cc File media/filters/chunk_demuxer_unittest.cc (right): https://codereview.chromium.org/1349973002/diff/1/media/filters/chunk_demuxer_unittest.cc#newcode3415 media/filters/chunk_demuxer_unittest.cc:3415: // GC should be able to evict frames in ...
3 years, 6 months ago (2015-09-18 01:29:28 UTC) #3
wolenetz
On 2015/09/18 01:29:28, servolk wrote: > https://codereview.chromium.org/1349973002/diff/1/media/filters/chunk_demuxer_unittest.cc > File media/filters/chunk_demuxer_unittest.cc (right): > > https://codereview.chromium.org/1349973002/diff/1/media/filters/chunk_demuxer_unittest.cc#newcode3415 > ...
3 years, 6 months ago (2015-09-21 19:49:56 UTC) #4
wolenetz
Also, regarding: "Wait, as far as I understand GOP handling is already done by FreeBuffers ...
3 years, 6 months ago (2015-09-21 20:23:13 UTC) #5
servolk
3 years, 6 months ago (2015-09-21 20:29:56 UTC) #6
On 2015/09/21 20:23:13, wolenetz wrote:
> Also, regarding:
> "Wait, as far as I understand GOP handling is already done by FreeBuffers
> (which uses SourceBufferRange::DeleteGOP* methods, so we always delete
> GOP at a time, i.e. key frame and all data that depends on it). I'm not
> sure how this should work in cases where newly appended segments are
> allowed to start with non-keyframes. I agree that for now this logic is
> probably ok, since we don't currently allow that."
> 
> We do collect GOP-at-a-time, but we don't know at GC time if there is the next
> append continues a GOP that was only partially appended in the last append.
This
> isn't a change from previous code, so addressing this situation is outside the
> scope of your GC-seek-fixing CL(s) like this one and the main one.
> Essentially, I might need to make the coded frame processor more aware of
> whenever the last appended GOP for a track is removed (either by GC or
explicit
> removal), so that it has the option of also triggering discontinuity detection
> if the next-append for that track appears adjacent but doesn't begin with a
> keyframe (bug 229412's fix should include this logic or similar eventually).

Yeah, ok, got it. Indeed we don't know during GC if there is another append
coming in the future that it going to complete the GOP. But you are right that
that's outside of the scope of this CL.

Powered by Google App Engine
This is Rietveld 408576698