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

[Reimar Döffinger]

On Sun, Sep 28, 2014 at 01:52:45PM +0200, wm4 wrote:
> On Sun, 28 Sep 2014 13:37:59 +0200
> Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > > 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.
> 
> All you need is something like av_format_flush(ctx).

To be honest I'm kind of shocked we don't have it.
I am bit surprised that things even work as well as they do,
MPlayer kind of assumes such a function is implemented in the demuxer.

> So the question is: what do you do if your ts (or anything else) is
> actually in a seekable file, and you don't want libavformat to invoke
> the read_seek callback? This is not possible without having full
> knowledge which formats call read_seek "just because", and which really
> need it for working seeking.

I admit I would have assumed that you'd always want them to use read_seek if the
stream layer provides one, including when they can seek (mostly?) fine
without.

> > 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.
> 
> But this is the more common case...

Yes, but it wasn't what I thought you were referring to, so
misunderstanding here.

> > 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.
> 
> Probably, but note that MPlayer-style byte caches can be read and
> seeked even if the underlying stream is stuck and blocked. And if that
> is not an issue, why have a cache at all?

Because it is quite important for formats like MOV/MP4 where you
constantly seek during playback, there every millisecond matters.
For the seek itself I assumed that a few milliseconds extra are not
a big deal.

> > > 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?).
> 
> What are you talking about? My MPlayer fork uses libavformat with
> libbluray just fine, while MPlayer doesn't and _can't_.

Actually I think MPlayer works mostly fine with it now, and admittedly
without this patch.
Except for it missing that one hack for flushing. Am I missing something
why you claim it can't?

> The only "hack" about this is that libavformat doesn't provide a proper
> flush function. It could be easily added.

Do you volunteer? :-)

> > >> 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.
> 
> I just suggested a clean solution. If you don't want to listen, fine,
> pursue your shitty hacks to keep an aging codebase working.

See, that's where our disagreement comes from: I considered your
approach the "shitty hack" (well, not quite that much, but a bit).
I'll not completely revise that until we have a proper flush function.
:)
I also still think it kind of would be nice if anyone could implement
a protocol that can help with seeking just by providing a time seek function
instead of hacking every single application to manually call a stream
seek.
But with the problems you mention and it being pointless for MPlayer on
top it might be better to deal with it when someone has a use for it.
So I'll retract the patch.
Btw.: mmsh.c seems to have a read_seek, too.

Reimar

P.S.: I hope you forgive me for the flinging of terms like "crap" and
"hack", I think I was too extreme there, even ignoring the parts
where I was wrong.