Add WebMediaPlayer::timelineOffset() support to WebMediaPlayerImpl.

Add WebMediaPlayer::timelineOffset() support to WebMediaPlayerImpl.

These changes add support for the
WebMediaPlayer::timelineOffset() method so that
HTMLMediaElement::getStartDate() can report this
information to JavaScript. FFmpegDemuxer & ChunkDemuxer
have been updated to provide "timeline offset" information
for WebM.

BUG=312699
TESTS=PipelineIntegrationTest.BasicPlayback, PipelineIntegrationTest.BasicPlaybackLive, PipelineIntegrationTest.BasicPlayback_MediaSource, PipelineIntegrationTest, BasicPlayback_MediaSource_Live

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264145

[scherkus (not reviewing), 2014-04-15 00:53:40]

lgtm w/ q's and nits

https://codereview.chromium.org/236023003/diff/1/media/base/demuxer.h
File media/base/demuxer.h (right):

https://codereview.chromium.org/236023003/diff/1/media/base/demuxer.h#newcode90
media/base/demuxer.h:90: // Returns wallclock time represented by presentation
timestamp 0.
isn't it "wall clock" with a space, as in a clock that is on the wall?

also it's interesting that you use timeline_offset_ in numerous places without
the wallclock_ prefix, but have functions that do have Wallclock in there

how about side-stepping this whole bit by removing the words "wall clock"
everywhere since base::Time is by definition a wall clock time? (see
base/time/time.h)

https://codereview.chromium.org/236023003/diff/1/media/base/stream_parser.h
File media/base/stream_parser.h (right):

https://codereview.chromium.org/236023003/diff/1/media/base/stream_parser.h#n...
media/base/stream_parser.h:58: // Third parameter - Indicates the wallclock time
associated with
ditto



side note: I wonder whether these large blocks of comments could be replaced by
simple structs...

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer.cc
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer...
media/filters/ffmpeg_demuxer.cc:38: static base::Time
ExtractTimelineOffset(AVFormatContext* format_context) {
parsing code without tests makes me nervous

how about moving this to media/ffmpeg/ffmpeg_common.h and adding tests?

perhaps something like:
base::Time ConvertToTime(const char* ffmpeg_utc) { ... }

then you can keep the demuxer-related logic (e.g,. checking for webm/matroska,
getting the dictionary stuff) in ffmpeg_demuxer.cc

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer...
media/filters/ffmpeg_demuxer.cc:44: // Convert ffmpeg UTC representation
(YYYY-MM-DD HH:MM:SS) to base::Time.
s/ffmpeg/FFmpeg/

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer...
media/filters/ffmpeg_demuxer.cc:51: // TODO: Update this parsing code when
FFmpeg returns sub-second
TODO(acolwell)?

https://codereview.chromium.org/236023003/diff/1/media/filters/pipeline_integ...
File media/filters/pipeline_integration_test.cc (right):

https://codereview.chromium.org/236023003/diff/1/media/filters/pipeline_integ...
media/filters/pipeline_integration_test.cc:623: 
remove extra blink

https://codereview.chromium.org/236023003/diff/1/media/formats/webm/webm_info...
File media/formats/webm/webm_info_parser.cc (right):

https://codereview.chromium.org/236023003/diff/1/media/formats/webm/webm_info...
media/formats/webm/webm_info_parser.cc:85: base::Time::Exploded exploded_epoc;
s/epoc/epoch/?

[acolwell GONE FROM CHROMIUM, 2014-04-16 01:01:08]

https://codereview.chromium.org/236023003/diff/1/media/base/demuxer.h
File media/base/demuxer.h (right):

https://codereview.chromium.org/236023003/diff/1/media/base/demuxer.h#newcode90
media/base/demuxer.h:90: // Returns wallclock time represented by presentation
timestamp 0.
On 2014/04/15 00:53:40, scherkus wrote:
> isn't it "wall clock" with a space, as in a clock that is on the wall?
> 
> also it's interesting that you use timeline_offset_ in numerous places without
> the wallclock_ prefix, but have functions that do have Wallclock in there
> 
> how about side-stepping this whole bit by removing the words "wall clock"
> everywhere since base::Time is by definition a wall clock time? (see
> base/time/time.h)

Done.

https://codereview.chromium.org/236023003/diff/1/media/base/stream_parser.h
File media/base/stream_parser.h (right):

https://codereview.chromium.org/236023003/diff/1/media/base/stream_parser.h#n...
media/base/stream_parser.h:58: // Third parameter - Indicates the wallclock time
associated with
On 2014/04/15 00:53:40, scherkus wrote:
> ditto
Removed wallclock reference.

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer.cc
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer...
media/filters/ffmpeg_demuxer.cc:38: static base::Time
ExtractTimelineOffset(AVFormatContext* format_context) {
On 2014/04/15 00:53:40, scherkus wrote:
> parsing code without tests makes me nervous
> 
> how about moving this to media/ffmpeg/ffmpeg_common.h and adding tests?
> 
> perhaps something like:
> base::Time ConvertToTime(const char* ffmpeg_utc) { ... }
> 
> then you can keep the demuxer-related logic (e.g,. checking for webm/matroska,
> getting the dictionary stuff) in ffmpeg_demuxer.cc

Done.

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer...
media/filters/ffmpeg_demuxer.cc:44: // Convert ffmpeg UTC representation
(YYYY-MM-DD HH:MM:SS) to base::Time.
On 2014/04/15 00:53:40, scherkus wrote:
> s/ffmpeg/FFmpeg/

Done.

https://codereview.chromium.org/236023003/diff/1/media/filters/ffmpeg_demuxer...
media/filters/ffmpeg_demuxer.cc:51: // TODO: Update this parsing code when
FFmpeg returns sub-second
On 2014/04/15 00:53:40, scherkus wrote:
> TODO(acolwell)?

Done.

https://codereview.chromium.org/236023003/diff/1/media/filters/pipeline_integ...
File media/filters/pipeline_integration_test.cc (right):

https://codereview.chromium.org/236023003/diff/1/media/filters/pipeline_integ...
media/filters/pipeline_integration_test.cc:623: 
On 2014/04/15 00:53:40, scherkus wrote:
> remove extra blink

Done.

https://codereview.chromium.org/236023003/diff/1/media/formats/webm/webm_info...
File media/formats/webm/webm_info_parser.cc (right):

https://codereview.chromium.org/236023003/diff/1/media/formats/webm/webm_info...
media/formats/webm/webm_info_parser.cc:85: base::Time::Exploded exploded_epoc;
On 2014/04/15 00:53:40, scherkus wrote:
> s/epoc/epoch/?

Done.

[acolwell GONE FROM CHROMIUM, 2014-04-16 01:01:12]

The CQ bit was checked by acolwell@chromium.org

[commit-bot: I haz the power, 2014-04-16 01:03:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/acolwell@chromium.org/236023003/20001

[commit-bot: I haz the power, 2014-04-16 01:38:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/acolwell@chromium.org/236023003/20001

[commit-bot: I haz the power, 2014-04-16 07:15:15]

Change committed as 264145