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

[Reimar Döffinger]

On 28.09.2014, at 11:05, wm4 <nfxjfg at googlemail.com> wrote:
> 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.

Which as you mention later is only possible with messy hacks.

>>> 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.

That is in no way different from implementing it in MPlayer, unless you compared doing a hack of only calling stream seek if the stream type is bluray but not doing that optimization in the read_seek function.
But then the speed difference is because you compare different implementations.
Also this only applies to cases where the read_seek is not supported.
In other cases the performance is unchanged.
Also it only doubles the round-trip latency which if that really is significant is an issue that needs to be fixed at the source.

>> 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.)

And this horribly broken, unreliable, crappy hack is supposed to be a better "solution"?
The absolute minimum for me to consider that acceptable would be a working, tested, official resync function (we really don't have that yet for demuxers?).

>> 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.

Because I want FFmpeg to work well, not make both projects a crap heap by adding workarounds in one around issues in the other.