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

Issue 10581050: Ensure media's buffered ranges always have a range that includes currentTime. (Closed)

Created:
8 years, 6 months ago by Ami GONE FROM CHROMIUM
Modified:
8 years, 6 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org
Visibility:
Public.

Description

Ensure media's buffered ranges always have a range that includes currentTime. Avoids buffering bar disappearing/reappearing when the bytes are distributed unevenly throughout the media. BUG=133567, 131444 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=143765

Patch Set 1 : . #

Total comments: 1

Patch Set 2 : . #

Patch Set 3 : Alternative implementation, introducing DataSourceHost::AddBufferedTimeRange(). #

Total comments: 16

Patch Set 4 : . #

Patch Set 5 : . #

Patch Set 6 : . #

Total comments: 2

Patch Set 7 : . #

Patch Set 8 : . #

Patch Set 9 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+144 lines, -45 lines) Patch
M media/base/data_source.h View 1 2 2 chunks +7 lines, -0 lines 0 comments Download
M media/base/demuxer_stream.h View 1 2 3 4 5 6 2 chunks +4 lines, -0 lines 0 comments Download
M media/base/mock_data_source_host.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M media/base/mock_demuxer_host.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M media/base/mock_filters.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M media/base/pipeline.h View 1 2 2 chunks +3 lines, -0 lines 0 comments Download
M media/base/pipeline.cc View 1 2 2 chunks +12 lines, -0 lines 0 comments Download
M media/base/pipeline_unittest.cc View 1 2 3 4 5 6 2 chunks +13 lines, -0 lines 0 comments Download
M media/base/ranges.h View 1 2 4 chunks +14 lines, -1 line 0 comments Download
A media/base/ranges.cc View 1 2 1 chunk +15 lines, -0 lines 0 comments Download
M media/filters/chunk_demuxer.cc View 1 2 3 4 5 6 8 chunks +10 lines, -28 lines 0 comments Download
M media/filters/chunk_demuxer_unittest.cc View 1 2 3 4 5 6 5 chunks +7 lines, -11 lines 0 comments Download
M media/filters/dummy_demuxer.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M media/filters/dummy_demuxer.cc View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M media/filters/ffmpeg_demuxer.h View 1 2 3 4 5 6 4 chunks +8 lines, -1 line 0 comments Download
M media/filters/ffmpeg_demuxer.cc View 1 2 3 4 5 6 7 5 chunks +37 lines, -3 lines 0 comments Download
M media/filters/ffmpeg_demuxer_unittest.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M media/media.gyp View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M media/tools/seek_tester/seek_tester.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
Ami GONE FROM CHROMIUM
8 years, 6 months ago (2012-06-20 23:46:10 UTC) #1
scherkus (not reviewing)
What's the behaviour when seeking where currentTime falls inbetween buffered_byte_ranges_.end(0) and buffered_byte_ranges_.start(1)? https://chromiumcodereview.appspot.com/10581050/diff/2001/media/base/pipeline.cc File media/base/pipeline.cc ...
8 years, 6 months ago (2012-06-21 01:10:38 UTC) #2
Ami GONE FROM CHROMIUM
Done & done.
8 years, 6 months ago (2012-06-21 02:44:37 UTC) #3
scherkus (not reviewing)
https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/chunk_demuxer.cc File media/filters/chunk_demuxer.cc (left): https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/chunk_demuxer.cc#oldcode83 media/filters/chunk_demuxer.cc:83: enum { kFakeTotalBytes = 1000000 }; \m/, https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc File ...
8 years, 6 months ago (2012-06-21 21:57:04 UTC) #4
Ami GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc File media/filters/ffmpeg_demuxer.cc (right): https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc#newcode99 media/filters/ffmpeg_demuxer.cc:99: base::TimeDelta(), ConvertStreamTimestamp( On 2012/06/21 21:57:04, scherkus wrote: > this ...
8 years, 6 months ago (2012-06-21 22:49:02 UTC) #5
scherkus (not reviewing)
https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc File media/filters/ffmpeg_demuxer.cc (right): https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc#newcode99 media/filters/ffmpeg_demuxer.cc:99: base::TimeDelta(), ConvertStreamTimestamp( On 2012/06/21 22:49:02, Ami Fischman wrote: > ...
8 years, 6 months ago (2012-06-21 22:58:22 UTC) #6
Ami GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc File media/filters/ffmpeg_demuxer.cc (right): https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.cc#newcode99 media/filters/ffmpeg_demuxer.cc:99: base::TimeDelta(), ConvertStreamTimestamp( On 2012/06/21 22:58:22, scherkus wrote: > On ...
8 years, 6 months ago (2012-06-21 23:18:11 UTC) #7
scherkus (not reviewing)
https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.h File media/filters/ffmpeg_demuxer.h (right): https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.h#newcode56 media/filters/ffmpeg_demuxer.h:56: FFmpegDemuxerStream(const NotifyBufferedCB& notify_buffered_cb_, On 2012/06/21 22:49:02, Ami Fischman wrote: ...
8 years, 6 months ago (2012-06-21 23:18:42 UTC) #8
Ami GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.h File media/filters/ffmpeg_demuxer.h (right): https://chromiumcodereview.appspot.com/10581050/diff/14001/media/filters/ffmpeg_demuxer.h#newcode56 media/filters/ffmpeg_demuxer.h:56: FFmpegDemuxerStream(const NotifyBufferedCB& notify_buffered_cb_, On 2012/06/21 23:18:42, scherkus wrote: > ...
8 years, 6 months ago (2012-06-22 03:55:47 UTC) #9
scherkus (not reviewing)
https://chromiumcodereview.appspot.com/10581050/diff/18002/media/filters/ffmpeg_demuxer.cc File media/filters/ffmpeg_demuxer.cc (right): https://chromiumcodereview.appspot.com/10581050/diff/18002/media/filters/ffmpeg_demuxer.cc#newcode725 media/filters/ffmpeg_demuxer.cc:725: buffered_times_[stream_id].Add(start, end); I think each FDS stream should hold ...
8 years, 6 months ago (2012-06-22 06:13:51 UTC) #10
Ami GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/10581050/diff/18002/media/filters/ffmpeg_demuxer.cc File media/filters/ffmpeg_demuxer.cc (right): https://chromiumcodereview.appspot.com/10581050/diff/18002/media/filters/ffmpeg_demuxer.cc#newcode725 media/filters/ffmpeg_demuxer.cc:725: buffered_times_[stream_id].Add(start, end); On 2012/06/22 06:13:51, scherkus wrote: > I ...
8 years, 6 months ago (2012-06-22 17:15:32 UTC) #11
scherkus (not reviewing)
lgtm
8 years, 6 months ago (2012-06-22 18:14:17 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fischman@chromium.org/10581050/26001
8 years, 6 months ago (2012-06-22 18:23:19 UTC) #13
commit-bot: I haz the power
Try job failure for 10581050-26001 (retry) on mac_rel for step "compile" (clobber build). It's a ...
8 years, 6 months ago (2012-06-22 18:39:21 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fischman@chromium.org/10581050/34001
8 years, 6 months ago (2012-06-22 22:44:37 UTC) #15
commit-bot: I haz the power
8 years, 6 months ago (2012-06-23 01:04:55 UTC) #16
Change committed as 143765

Powered by Google App Engine
This is Rietveld 408576698