Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(989)

Issue 10692053: Remove buffer duration calculation from WebMClusterParser and update SourceBufferStream accordingly (Closed)

Created:
8 years, 5 months ago by vrk (LEFT CHROMIUM)
Modified:
8 years, 5 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org
Visibility:
Public.

Description

Remove buffer duration calculation from WebMClusterParser and update SourceBufferStream accordingly This removes the work to calculate buffers' durations in WebMClusterParser and modifies SourceBufferStream to no longer need duration information to work. This allows us to fix several bugs. BUG=131695, 136418 TEST=media_unittests, go/oiedsb no longer needs Abort()s to work Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145839

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 28

Patch Set 4 : Response to CR #

Patch Set 5 : . #

Patch Set 6 : Rebase ToT #

Total comments: 29

Patch Set 7 : respond to CR #

Patch Set 8 : Rebase ToT + fix clang errors #

Unified diffs Side-by-side diffs Delta from patch set Stats (+614 lines, -475 lines) Patch
M media/base/stream_parser_buffer.h View 1 2 3 4 5 1 chunk +0 lines, -3 lines 0 comments Download
M media/base/stream_parser_buffer.cc View 1 2 3 4 5 2 chunks +0 lines, -7 lines 0 comments Download
M media/filters/chunk_demuxer.cc View 5 chunks +4 lines, -18 lines 0 comments Download
M media/filters/chunk_demuxer_unittest.cc View 1 2 3 4 5 6 7 10 chunks +43 lines, -74 lines 0 comments Download
M media/filters/source_buffer_stream.h View 1 2 3 4 5 6 5 chunks +58 lines, -12 lines 0 comments Download
M media/filters/source_buffer_stream.cc View 1 2 3 4 5 6 27 chunks +333 lines, -144 lines 0 comments Download
M media/filters/source_buffer_stream_unittest.cc View 1 2 3 4 5 6 7 60 chunks +168 lines, -131 lines 0 comments Download
M media/webm/webm_cluster_parser.h View 1 2 3 1 chunk +0 lines, -12 lines 0 comments Download
M media/webm/webm_cluster_parser.cc View 1 2 3 4 5 6 2 chunks +8 lines, -74 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
vrk (LEFT CHROMIUM)
I also tried to simplify some overlap stuff (the UpdateTrackBuffer() business is all in one ...
8 years, 5 months ago (2012-06-30 00:04:19 UTC) #1
vrk (LEFT CHROMIUM)
Ah, also: this CL is not yet committable because I need to update the test ...
8 years, 5 months ago (2012-06-30 00:08:40 UTC) #2
acolwell GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/10692053/diff/1011/media/filters/chunk_demuxer_unittest.cc File media/filters/chunk_demuxer_unittest.cc (right): https://chromiumcodereview.appspot.com/10692053/diff/1011/media/filters/chunk_demuxer_unittest.cc#newcode49 media/filters/chunk_demuxer_unittest.cc:49: static const char* kDefaultFirstClusterRange = "{ [0,23) }"; ? ...
8 years, 5 months ago (2012-07-02 17:12:32 UTC) #3
vrk (LEFT CHROMIUM)
Thanks Aaron! https://chromiumcodereview.appspot.com/10692053/diff/1011/media/filters/chunk_demuxer_unittest.cc File media/filters/chunk_demuxer_unittest.cc (right): https://chromiumcodereview.appspot.com/10692053/diff/1011/media/filters/chunk_demuxer_unittest.cc#newcode49 media/filters/chunk_demuxer_unittest.cc:49: static const char* kDefaultFirstClusterRange = "{ [0,23) ...
8 years, 5 months ago (2012-07-04 02:50:18 UTC) #4
acolwell GONE FROM CHROMIUM
LGTM % nits https://chromiumcodereview.appspot.com/10692053/diff/7003/media/filters/source_buffer_stream.cc File media/filters/source_buffer_stream.cc (right): https://chromiumcodereview.appspot.com/10692053/diff/7003/media/filters/source_buffer_stream.cc#newcode22 media/filters/source_buffer_stream.cc:22: typedef base::Callback<base::TimeDelta()> InterbufferDistanceCB; nit: Add comment ...
8 years, 5 months ago (2012-07-09 18:15:54 UTC) #5
vrk (LEFT CHROMIUM)
Thanks for reviewing! https://chromiumcodereview.appspot.com/10692053/diff/7003/media/filters/source_buffer_stream.cc File media/filters/source_buffer_stream.cc (right): https://chromiumcodereview.appspot.com/10692053/diff/7003/media/filters/source_buffer_stream.cc#newcode22 media/filters/source_buffer_stream.cc:22: typedef base::Callback<base::TimeDelta()> InterbufferDistanceCB; On 2012/07/09 18:15:55, ...
8 years, 5 months ago (2012-07-10 00:05:33 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vrk@chromium.org/10692053/24005
8 years, 5 months ago (2012-07-10 03:24:16 UTC) #7
commit-bot: I haz the power
8 years, 5 months ago (2012-07-10 04:41:22 UTC) #8
Change committed as 145839

Powered by Google App Engine
This is Rietveld 408576698