Support setting an explicit duration on MediaSource objects

Support setting an explicit duration on MediaSource objects

This is the Chromium-side change to support MediaSource's duration attribute.

BUG=NONE


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=153613

[vrk (LEFT CHROMIUM), 2012-08-24 01:27:29]

Still in the procress of working out the WebKit side but here's the Chromium
side for now!

[acolwell GONE FROM CHROMIUM, 2012-08-24 18:19:10]

Overall looks good. Just a few comments.

http://codereview.chromium.org/10879056/diff/11/media/filters/chunk_demuxer.cc
File media/filters/chunk_demuxer.cc (right):

http://codereview.chromium.org/10879056/diff/11/media/filters/chunk_demuxer.c...
media/filters/chunk_demuxer.cc:838: UpdateDuration(duration);
I think you need a 
if(duration != duration_)
  return;

otherwise I believe 'mediaSource.duration = mediaSource.duration;' in JavaScript
will DCHECK.

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
File media/filters/source_buffer_stream.cc (right):

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:780: // If we're about to delete the
selected range, also reset the seek state.
DCHECK_GE((*itr)->GetStartTimestamp(), duration)?

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:1125: base::TimeDelta timestamp,
BufferQueue* removed_buffers) {
Remove |remove_buffers| since there is only one caller for this method and NULL
is always passed for remove_buffers.

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
File media/filters/source_buffer_stream_unittest.cc (right):

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
media/filters/source_buffer_stream_unittest.cc:2006: // Appending data at
position 10 should not fulfill the seek.
Based on our offline conversation could you please clarify this comment to
indicate why the seek shouldn't be fulfilled.

http://codereview.chromium.org/10879056/diff/11/webkit/media/webmediaplayer_i...
File webkit/media/webmediaplayer_impl.cc (right):

http://codereview.chromium.org/10879056/diff/11/webkit/media/webmediaplayer_i...
webkit/media/webmediaplayer_impl.cc:669: if (static_cast<double>(duration()) ==
new_duration)
if duration() is a float should we have new_duration also be a float?

[vrk (LEFT CHROMIUM), 2012-08-28 00:03:22]

Also uploaded the WebKit side:
https://bugs.webkit.org/show_bug.cgi?id=95149

http://codereview.chromium.org/10879056/diff/11/media/filters/chunk_demuxer.cc
File media/filters/chunk_demuxer.cc (right):

http://codereview.chromium.org/10879056/diff/11/media/filters/chunk_demuxer.c...
media/filters/chunk_demuxer.cc:838: UpdateDuration(duration);
On 2012/08/24 18:19:10, acolwell wrote:
> I think you need a 
> if(duration != duration_)
>   return;
> 
> otherwise I believe 'mediaSource.duration = mediaSource.duration;' in
JavaScript
> will DCHECK.

Did == instead :)

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
File media/filters/source_buffer_stream.cc (right):

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:780: // If we're about to delete the
selected range, also reset the seek state.
On 2012/08/24 18:19:10, acolwell wrote:
> DCHECK_GE((*itr)->GetStartTimestamp(), duration)?

Done.

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:1125: base::TimeDelta timestamp,
BufferQueue* removed_buffers) {
On 2012/08/24 18:19:10, acolwell wrote:
> Remove |remove_buffers| since there is only one caller for this method and
NULL
> is always passed for remove_buffers.

Done.

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
File media/filters/source_buffer_stream_unittest.cc (right):

http://codereview.chromium.org/10879056/diff/11/media/filters/source_buffer_s...
media/filters/source_buffer_stream_unittest.cc:2006: // Appending data at
position 10 should not fulfill the seek.
On 2012/08/24 18:19:10, acolwell wrote:
> Based on our offline conversation could you please clarify this comment to
> indicate why the seek shouldn't be fulfilled.

Done.

http://codereview.chromium.org/10879056/diff/11/webkit/media/webmediaplayer_i...
File webkit/media/webmediaplayer_impl.cc (right):

http://codereview.chromium.org/10879056/diff/11/webkit/media/webmediaplayer_i...
webkit/media/webmediaplayer_impl.cc:669: if (static_cast<double>(duration()) ==
new_duration)
On 2012/08/24 18:19:10, acolwell wrote:
> if duration() is a float should we have new_duration also be a float?

I think duration() should be a double actually; looks like the
HTMLMediaElement's duration is a double:
http://dev.w3.org/html5/spec/media-elements.html#dom-media-duration
but it's a float in the IDL file/webkit code. Will ask Eric about it.

[acolwell GONE FROM CHROMIUM, 2012-08-28 00:23:16]

LGTM

[commit-bot: I haz the power, 2012-08-28 00:42:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vrk@chromium.org/10879056/21

[commit-bot: I haz the power, 2012-08-28 03:07:35]

Change committed as 153613