[FFmpeg-devel] [PATCH 3/3] [RFC] mpegts: Support seeking based on stream timestamps.

[wm4]

On Sun, 28 Sep 2014 10:40:18 +0200
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On Sun, Sep 28, 2014 at 10:15:51AM +0200, wm4 wrote:
> > On Sun, 21 Sep 2014 10:17:16 +0100
> > Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > > @@ -2680,6 +2687,7 @@ AVInputFormat ff_mpegtsraw_demuxer = {
> > >      .read_packet    = mpegts_raw_read_packet,
> > >      .read_close     = mpegts_read_close,
> > >      .read_timestamp = mpegts_get_dts,
> > > +    .read_seek      = mpegts_read_seek,
> > >      .flags          = AVFMT_SHOW_IDS | AVFMT_TS_DISCONT,
> > >      .priv_class     = &mpegtsraw_class,
> > >  };
> > 
> > IMO this is not a good idea. Seeking should seek the stream to a
> > timestamp; but the demuxer will output mismatching timestamps with a
> > different offset!
> 
> If you combine a stream layer with different timestamps, yes.

That's not how the libavformat seeking API works. If you want a
different layer, use something from the different layer. E.g. seek avio
directly, and flush the demuxer's buffers.

> > And in general, this seems useless: if you want to fix MPlayer, what
> > stops you from redirecting seeks to the stream layer if needed
> > directly in demux_lavf.c's seek function?
> 
> I want to fix FFmpeg. If it has a function to seek based on the
> stream timestamps it should be using it!

But it doesn't work! If you seek to a timestamp using libbluray, the
resulting mpeg-ts packets can have a completely different timestamp. I
see something like a layering violation here.

> > In general, the application
> > can decide much better whether it wants to seek using the packet
> > timestamps or the stream layer's special seek function.
> 
> It can do that very easily: just return an error from the stream's
> seek function/do not implement it at all.
> 
> > Also, in the
> > context of MPlayer, your patch might actually trigger slow operations
> > when playing a .ts file with cache enabled: it will have to do a
> > synchronous call through the stream cache layer to call the seek
> > function.
> 
> How is that in any way different from your proposal that fixes it
> in MPlayer?
> Also that operation isn't any slower than normal seeking (assuming
> the seek is not within the cache).

Huh? With your patch, MPlayer would send a seek stream ctrl to the
cache, and would have to do a blocking wait for the cache process. It
can't know whether the implementation is even implemented before
sending the stream ctrl.

Anyway, if this patch is applied, I can easily workaround this in my
own code by checking whether rtmp is used to avoid this situation, so
it's not a blocker for me.

> > If we want to fix FFmpeg's native libbluray support, I think we should
> > export it as demuxer, which opens a .ts demuxer as "slave", and
> > redirects the seek operations correctly to libbluray properly. It also
> > could rewrite the timestamps to reflect actual playback time, so that
> > seeking to a time actually works as expected. (Plus removes horrible
> > hacks from players to display the playback time correctly.)
> 
> This fixes only one specific single case and it doesn't help an
> application that wants to combine its own bluray handling (for example
> using something other than libbluray) with FFmpeg's demuxer, they
> still need to hack up their own seeking code for that and then
> beat FFmpeg it not seeking on its own, and manually reset the demuxer
> etc.

Absolutely nothing stops the application from doing its own seeking for
formats like mpeg-ts. Though what you need is a function that flushes
internal libavformat buffers to make sure no old data is read after the
stream-level seek was performed. (Currently I execute a byte seek to
the current position to achieve this flushing.)

> > In general, this avio_read_seek() thing seems to be a somewhat
> > misguided attempt to coerce high level streaming protocols into the low
> > level byte-oriented "protocol" layer. Apparently it's only used by
> > rtmp, which does scary thing like remuxing the received data to flv,
> > just to bypass the protocol/demuxer separation. Newer protocol
> > implementations like HLS don't do this anymore, and just put everything
> > into the demuxer.
> 
> And instead don't allow for any kind of proper caching.

You can cache packets.

> Not exactly my definition of a better design.

I'm inclined to claim that remuxing to FLV makes a pretty terrible
implementation. Why not return the packets directyly!?

> Either way the read_seek is there, and until such time someone removes
> it I am not inclined to consider it a good idea to keep it broken!

It's not broken. If the fact that mpeg-ts doesn't use it means it's
broken, you're going to have to apply the same patch to a lot of
demuxers.

And again, I don't understand why you can't just redirect the seek in
demux_lavf.c. This would be an easy fix for the MPlayer problem.