Seek for video track changes on FFmpegDemuxer.

Seek for video track changes on FFmpegDemuxer.

Prototype fix for seeking when doing a track change on src=. Mostly
works, but does not resolve the following edges:

- If there's a gap between last read audio frame and seeked video
  point, we'll lose audio - fix is likely to use same logic as Seek()
  instead of using |selected_stream| for the seek index.
- Needs a better mechanism for ignoring already demuxed packets so
  that we don't get repetitions in the audio. There are "valid"
  files with out of order timestamps, so care must be taken not
  to break those.
- Does not handle errors which occur during seeking.

* NOT READY FOR SUBMISSION *

BUG=709302
TEST=basic fg/bg with bg opt works correctly.

[DaleCurtis, 2017-04-11 00:49:21]

Description was changed from

==========
Seek for video track changes on FFmpegDemuxer.

Prototype fix for seeking when doing a track change on src=. Mostly
works, but does not resolve the following edges:

- If there's a gap between last read audio frame and seeked video
  point, we'll lose audio - fix is likely to use same logic as Seek()
  instead of using |selected_stream| for the seek index.
- Needs a better mechanism for ignoring already demuxed packets so
  that we don't get repetitions in the audio. There are "valid"
  files with out of order timestamps, so care must be taken not
  to break those.

BUG=709302
TEST=basic fg/bg with bg opt works correctly.
==========

to

==========
Seek for video track changes on FFmpegDemuxer.

Prototype fix for seeking when doing a track change on src=. Mostly
works, but does not resolve the following edges:

- If there's a gap between last read audio frame and seeked video
  point, we'll lose audio - fix is likely to use same logic as Seek()
  instead of using |selected_stream| for the seek index.
- Needs a better mechanism for ignoring already demuxed packets so
  that we don't get repetitions in the audio. There are "valid"
  files with out of order timestamps, so care must be taken not
  to break those.
- Does not handle errors which occur during seeking.

* NOT READY FOR SUBMISSION *

BUG=709302
TEST=basic fg/bg with bg opt works correctly.
==========

[servolk, 2017-04-29 00:04:08]

On 2017/04/11 00:49:24, DaleCurtis wrote:

Dale, thanks for putting together this CL! I have tested it and indeed the video
resumes much faster with it, since we resume playback as soon as video decoder
catches up with the current playback position, instead of having to wait for the
next key frame.
But I have a few follow up questions re the points you made in the CL
description:

> If there's a gap between last read audio frame and seeked video point, we'll
lose audio - fix is likely to use same logic as Seek() instead of using
|selected_stream| for the seek index.

I believe it's highly unlikely in properly muxed streams, since the seek should
go backward - to the nearest video key frame before the current playback
position. I guess it's possible that in badly muxed streams this might move the
stream read pointer forward and cause a gap in audio, but it's not clear to me
how we could mitigate that. I believe that even if we do a full-fledged
FFmpegDemuxer::Seek with video as preferred stream, we still have the same risk
for badly-muxed streams, or did I misunderstand something here?

> Needs a better mechanism for ignoring already demuxed packets so that we don't
get repetitions in the audio. There are "valid" files with out of order
timestamps, so care must be taken not to break those.

I don't have a lot of context on this - could you give an example of a stream
that would have an out-of-order timestamps? Are timestamps out of order only
when we initially receive them from ffmpeg or are they out-of-order even after
all the timestamp fixups and adjustments done in
FFmpegDemuxerStream::EnqueuePacket? Can audio buffers with out-of-order
timestamps reach the audio renderer? (i.e. does the audio renderer know how to
handle those cases, or do we handle that somewhere earlier in the media
pipeline?).

> Does not handle errors which occur during seeking.

Agree, but looking at
https://cs.chromium.org/chromium/src/media/filters/ffmpeg_demuxer.cc?rcl=dbec...
it seems to me that we don't handle errors well even during regular seeking :) I
think we should do at least something like this:
MEDIA_LOG << " seek failed: " << AVErrorToString(result);
I'll send a CL.

[servolk, 2017-04-29 00:12:02]

On 2017/04/29 00:04:08, servolk wrote:
> On 2017/04/11 00:49:24, DaleCurtis wrote:
> 
> Dale, thanks for putting together this CL! I have tested it and indeed the
video
> resumes much faster with it, since we resume playback as soon as video decoder
> catches up with the current playback position, instead of having to wait for
the
> next key frame.
> But I have a few follow up questions re the points you made in the CL
> description:
> 
> > If there's a gap between last read audio frame and seeked video point, we'll
> lose audio - fix is likely to use same logic as Seek() instead of using
> |selected_stream| for the seek index.
> 
> I believe it's highly unlikely in properly muxed streams, since the seek
should
> go backward - to the nearest video key frame before the current playback
> position. I guess it's possible that in badly muxed streams this might move
the
> stream read pointer forward and cause a gap in audio, but it's not clear to me
> how we could mitigate that. I believe that even if we do a full-fledged
> FFmpegDemuxer::Seek with video as preferred stream, we still have the same
risk
> for badly-muxed streams, or did I misunderstand something here?
> 
> > Needs a better mechanism for ignoring already demuxed packets so that we
don't
> get repetitions in the audio. There are "valid" files with out of order
> timestamps, so care must be taken not to break those.
> 
> I don't have a lot of context on this - could you give an example of a stream
> that would have an out-of-order timestamps? Are timestamps out of order only
> when we initially receive them from ffmpeg or are they out-of-order even after
> all the timestamp fixups and adjustments done in
> FFmpegDemuxerStream::EnqueuePacket? Can audio buffers with out-of-order
> timestamps reach the audio renderer? (i.e. does the audio renderer know how to
> handle those cases, or do we handle that somewhere earlier in the media
> pipeline?).
> 
> > Does not handle errors which occur during seeking.
> 
> Agree, but looking at
>
https://cs.chromium.org/chromium/src/media/filters/ffmpeg_demuxer.cc?rcl=dbec...
> it seems to me that we don't handle errors well even during regular seeking :)
I
> think we should do at least something like this:
> MEDIA_LOG << " seek failed: " << AVErrorToString(result);
> I'll send a CL.

Perhaps something like this? To handle the errors during seeking?
https://codereview.chromium.org/2845333003/

[DaleCurtis, 2017-05-01 22:57:48]

On 2017/04/29 at 00:04:08, servolk wrote:
> On 2017/04/11 00:49:24, DaleCurtis wrote:
> 
> Dale, thanks for putting together this CL! I have tested it and indeed the
video resumes much faster with it, since we resume playback as soon as video
decoder catches up with the current playback position, instead of having to wait
for the next key frame.
> But I have a few follow up questions re the points you made in the CL
description:
> 
> > If there's a gap between last read audio frame and seeked video point, we'll
lose audio - fix is likely to use same logic as Seek() instead of using
|selected_stream| for the seek index.
> 
> I believe it's highly unlikely in properly muxed streams, since the seek
should go backward - to the nearest video key frame before the current playback
position. I guess it's possible that in badly muxed streams this might move the
stream read pointer forward and cause a gap in audio, but it's not clear to me
how we could mitigate that. I believe that even if we do a full-fledged
FFmpegDemuxer::Seek with video as preferred stream, we still have the same risk
for badly-muxed streams, or did I misunderstand something here?

Unlikely won't cut it in this case, it is possible, so we need to make sure we
don't break these playbacks. If we use the full fledged seek ffmpeg will take
care of ensuring other streams are at the right place.

> 
> > Needs a better mechanism for ignoring already demuxed packets so that we
don't get repetitions in the audio. There are "valid" files with out of order
timestamps, so care must be taken not to break those.
> 
> I don't have a lot of context on this - could you give an example of a stream
that would have an out-of-order timestamps? Are timestamps out of order only
when we initially receive them from ffmpeg or are they out-of-order even after
all the timestamp fixups and adjustments done in
FFmpegDemuxerStream::EnqueuePacket? Can audio buffers with out-of-order
timestamps reach the audio renderer? (i.e. does the audio renderer know how to
handle those cases, or do we handle that somewhere earlier in the media
pipeline?).

chained ogg files will have out of order timestamps; b-frame based video will
have out of order timestamps at the demuxer level.

> 
> > Does not handle errors which occur during seeking.
> 
> Agree, but looking at
https://cs.chromium.org/chromium/src/media/filters/ffmpeg_demuxer.cc?rcl=dbec...
it seems to me that we don't handle errors well even during regular seeking :) I
think we should do at least something like this:
> MEDIA_LOG << " seek failed: " << AVErrorToString(result);
> I'll send a CL.

Thanks will take a look.