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

Issue 2085012: Reporting a more accurate buffered time for the video tag (Closed)

Created:
10 years, 1 month ago by vrk (LEFT CHROMIUM)
Modified:
9 years, 2 months ago
CC:
chromium-reviews, darin-cc_chromium.org, awong, scherkus (not reviewing), Alpha Left Google, fbarchard, PaweĊ‚ Hajdan Jr.
Visibility:
Public.

Description

Reporting a more accurate buffered time for the video tag Instead of showing that the entire file is buffered when the video first loads, this uses current time, current bytes, and buffered bytes to estimate buffered time.

Patch Set 1 #

Patch Set 2 : Fixed proposed changes from code review. #

Total comments: 2

Patch Set 3 : Deleting extra blank line. #

Total comments: 11

Patch Set 4 : Modifying update so that this is no longer a 2-side patch (step 1!) #

Patch Set 5 : Fixed a mistake in merging with new code #

Patch Set 6 : Did a rebase so that my diffs are not confused with someone else's #

Total comments: 2

Patch Set 7 : Added functions to mock objects to get tests to compile. #

Patch Set 8 : Modified tests to work with changes. #

Patch Set 9 : Added checks to fix some edgecase bugs. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -13 lines) Patch
M media/base/filter_host.h View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M media/base/mock_filter_host.h View 1 chunk +2 lines, -0 lines 0 comments Download
M media/base/pipeline.h View 1 chunk +1 line, -1 line 0 comments Download
M media/base/pipeline_impl.h View 1 2 3 4 3 chunks +12 lines, -1 line 0 comments Download
M media/base/pipeline_impl.cc View 1 2 3 3 chunks +29 lines, -10 lines 0 comments Download
M media/base/pipeline_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/filters/ffmpeg_demuxer.cc View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M media/filters/ffmpeg_demuxer_unittest.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M webkit/glue/webmediaplayer_impl.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M webkit/glue/webmediaplayer_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +14 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
vrk (LEFT CHROMIUM)
10 years, 1 month ago (2010-05-20 01:24:33 UTC) #1
Alpha Left Google
one last comment and this is good to go. Thanks for doing this! http://codereview.chromium.org/2085012/diff/3001/2005 File ...
10 years, 1 month ago (2010-05-20 01:46:54 UTC) #2
vrk (LEFT CHROMIUM)
http://codereview.chromium.org/2085012/diff/3001/2005 File media/base/pipeline_impl.h (right): http://codereview.chromium.org/2085012/diff/3001/2005#newcode161 media/base/pipeline_impl.h:161: On 2010/05/20 01:46:54, Alpha wrote: > nit: remove this ...
10 years, 1 month ago (2010-05-20 16:49:12 UTC) #3
Alpha Left Google
Awsome. LGTM.
10 years, 1 month ago (2010-05-20 18:45:38 UTC) #4
scherkus (not reviewing)
tiny nits http://codereview.chromium.org/2085012/diff/10001/11004 File media/base/pipeline_impl.h (right): http://codereview.chromium.org/2085012/diff/10001/11004#newcode93 media/base/pipeline_impl.h:93: virtual int64 GetCurrentReadPosition(); do these need to ...
10 years, 1 month ago (2010-05-20 19:28:27 UTC) #5
vrk (LEFT CHROMIUM)
http://codereview.chromium.org/2085012/diff/10001/11004 File media/base/pipeline_impl.h (right): http://codereview.chromium.org/2085012/diff/10001/11004#newcode93 media/base/pipeline_impl.h:93: virtual int64 GetCurrentReadPosition(); Turns out these can be private! ...
10 years, 1 month ago (2010-05-21 18:10:13 UTC) #6
Alpha Left Google
On 2010/05/21 18:10:13, Victoria Kirst wrote: > http://codereview.chromium.org/2085012/diff/10001/11004 > File media/base/pipeline_impl.h (right): > > http://codereview.chromium.org/2085012/diff/10001/11004#newcode93 ...
10 years, 1 month ago (2010-05-21 18:16:17 UTC) #7
scherkus (not reviewing)
LGTM w/ nits Alpha can probably land it for you :) http://codereview.chromium.org/2085012/diff/10001/11004 File media/base/pipeline_impl.h (right): ...
10 years, 1 month ago (2010-05-21 18:44:08 UTC) #8
fbarchard
10 years, 1 month ago (2010-05-21 20:59:50 UTC) #9
LGTM
Its not a perfect solution, because the purpose of showing what is buffered is
the user knows where they can seek quickly.  That would include the disk cache.
In the future, we should 'autobuffer' more but perhaps to the disk cache, not
memory.

Powered by Google App Engine
This is Rietveld 408576698