[vlc-devel] [PATCH] input/stream: do not unconditionally invalidate block on seek

[Rémi Denis-Courmont]

Le maanantaina 24. lokakuuta 2016, 10.38.03 EEST Filip Roséen a écrit :
> Hi Remi
> 
> On 2016-10-24 11:33, Rémi Denis-Courmont wrote:
> > Le maanantaina 24. lokakuuta 2016, 2.13.11 EEST Filip Roséen a écrit :
> > > If a seek request happens to refer to a position within the already
> > > read block, simply update the block so that it refers to the desired
> > > data.
> > > 
> > > There is really no need for us to discard the block, only to ask the
> > > underlying stream to give us a new one (when we have the data we want
> > > already).
> > 
> > I don´t see the point. I have spent a lot of time untangling the caching
> > from the generic byte stream reader, and this is just starting to bring
> > it back in and duplicate functionality.
> 
> I agree that we should keep the implementation simple and consise, and
> not duplicate functionality; but having the kind of support proposed
> is just a extremely minor subset of the functionality within other
> parts of the code..

Any reasonable cache implementation will optimize a short forward skip or 
read. That is the whole point of read-ahead caching. So this is completely 
pointless complexity.

> and I think it make sense not to re-read data
> (just as we have the functionality in place for peeking).

Yes. That´s what VLC and operating systems have caches for.

> One could even merge the peeking and block "cache" in
> `src/input/stream.c`, making the implementation even less complicated
> than it is now.

Support for peek and block is already in-there. Nothing else needs to be 
merged, and anything more will obviously make the code more and needlessly 
complicated.

> > If you need to skip forward, call vlc_stream_Read(..., NULL, ...) in the
> > demuxer or wherever else needed. That will work even if the stream is not
> > readable.
> 
> I guess we should have a major clean-up session, because we have
> demuxers (such as `modules/demux/mp4` and `modules/demux/avi`), and
> other entities, that does a lot of seeking back and forth, and for
> block-based accessors this is killing performance (really bad).

This is FUD. Bringing cache_block back instead of prefetch will not improve 
uninterleaved file playback. cache_block has no support for multiple tracks, 
and never had. It is much worse than prefetch for that purpose.

Besides, reading uninterleaved AVI through a network has always sucked in VLC. 
Even if you remove the prefetch and new HTTP plugins, cache_stream's support 
for multiple tracks will only fix it partially (and it will break what 
prefetch fixed such as high-bandwidth non-seeking streams). We know that from 
old user feedback.

There is only one definite way to fix playback of uninterleaved tracks: set up 
one byte stream per track (at least per active track). Using multiple tracks 
with a single stream is mathematically bound to eventually fail:

Take:
- T the number of active tracks or read offsets,
- N the available network bandwidth,
- S the stream bitrate, and
- L the seek latency.

You need to accumulate buffers of T * S  * L and then some, only to compensate 
for seeking. You can build up that buffer at best at a rate of: N - S.

So you need to buffer for a duration of at least: T * S * L / (N - S)
And allocate corresponding buffer space of: T * S² * L / (N - S)

Note that this goes out-of-hand if N nears S, or if S is large. And that 
assumes you can even estimate those values.

As for a pure random access formatted file, you have to play from a random 
access media. It is unfixable at software level.

> Sure, they can be fixed to not discard already read data, but I find
> that having simple management of blocks within `src/input/stream.c` is
> not too complex.

-- 
Rémi Denis-Courmont
Nonsponsored VLC developer
http://www.remlab.net/CV.pdf