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

[Filip Roséen]

Good evening Rémi,

On 2016-10-24 19:03, Rémi Denis-Courmont wrote:

> 	Kväll,
> 
> Le maanantaina 24. lokakuuta 2016, 17.22.52 EEST Filip Roséen a écrit :
> > I fail to see how you can say that it is *"[not] block-specific"*. The
> > path taken when reading blocks in `src/input/stream.c` is of course
> > different than the one reading data from streams exposing `pf_read`.
> 
> I say it s not block-specific because it is not block-specific. You said, more 
> or less, that the problem affects input streams that require excessively slow 
> seeking during normal playback:

That's not what I said in full, it affects *block-based* streams that
are being read as if they were *stream-based* given that they create
blocks that are then discarded (even though a seek refers to content
of the already available block).

> 
> | (...) we have demuxers (such as `modules/demux/mp4` and
> | `modules/demux/avi`), and other entities, that does (sic) a lot of seeking
> | back and forth, and for block-based accessors this is killing performance
> | (really bad).
> 
> Badly multiplexed MP4 and AVI files are certainly are not limited to block-
> based accesses.

See next comment.

> > To sum things up: We potentially regenerate/write/copy data simply
> > because `src/input/stream.c` discards it in `vlc_stream_Seek`;
> > 
> >  - you must agree with me on that at least?
> 
> Regenerate, yes, but it is not worth fixing because:
> 
> - The fix will intrinsically not work for stream-based accesses.

As already denoted, *stream-based* accessors are not suffering from
the same kind of problem given that they receive a request for `M`
bytes of data, not *"give me a block of whatever size you prefer"*.

> - The fix is intrinsically irrelevant for nonseekable accesses.

Yes, it is irrelevant for *nonseekable* streams, the discussion are
about the implications of calling `vlc_stream_Seek`.

> - The problem is, in practice, negligible for fastly seekable streams (but 
> then again, all of them are stream-based at the moment anyway).
> 
> - The problem is negligible for most demuxer plugins and for all properly 
> multiplexed files.
>
> - The prefetch cache filter already solves the problem for slowly seekable 
> streams. Specifically, the prefetch buffer capacity should exceed the size of 
> any reasonable single block. If not, the access should probably be reworked 
> because excessively large blocks will cause performance _other_ problems.

The modules responsible for prefetching/caching data does not apply to
*stream_filters* that are later in the *stream-chain* (which I have
already mentioned).

> - The fix is at the top of the slippery slope down to entangling caching and 
> generic I/O code.

Given that you are the author of the relevant implementation within
`src/input/stream.c` I am very surprised that you just said that,
especially given the following commit `f388861`.

What I would like is for us to have a common logic related to seeking
within an already created block, or peeked data. It is basically
equivalent to the implementation your wrote, so I, once again
honestly, do not see what argument you are making against such patch.

The current implementation can be simplified, a lot, if we just share
the code-path (which I mentioned in one my first replies in this
discussion).

> This patch is specific to pipelines using cache_block (or no cache at all) 
> with seekable stream and pathological multiplexing. To put stuff into 
> perspective, exactly zero valid combinations of access and demuxer plugins 
> would benefit from this patch in the current code base. Indeed, I believe only 
> the DVB and MMS access use cache_block at the moment. DVB is not seekable, and 
> MMS (ASF) does not normally require seeking.

It is not related to usage of *cache_block*, especially given that it
is too far up the stream to be of any importance (as I have already
described), it is however very much relevant to *stream_filters* that
are block-based (such as the soon to be proposed torrent module).

> So maybe I should call it premature optimization rather than micro-
> optimization.

Best Regards,\
Filip

>
> -- 
> Rémi Denis-Courmont
> Nonsponsored VLC developer
> http://www.remlab.net/CV.pdf
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161024/c04ed1a7/attachment.html>