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

[Filip Roséen]

Hi Rémi,

I do not have anything more to add to this discussion given that I
have exhausted all my ways of trying to get my point across, it might
be that we are not talking about the same thing, or that we are
talking on different frequencies.

For future reference, here's a summary
---------------------------------------------------------------------

In `src/input/stream.c:vlc_stream_Seek` we are discarding already
read data (inside a block) unconditionally when we seek, no matter
if the seek happens to refer to data that is already available
inside said block.
   
 - What should we do about this, if anything?

Best Regards,\
Filip

---------------------------------------------------------------------

---------------------------------------------------------------------

On 2016-10-24 20:43, Rémi Denis-Courmont wrote:

> Le maanantaina 24. lokakuuta 2016, 18.28.31 EEST Filip Roséen a écrit :
> > 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).
> 
> I don´t see a problem there as such. As explained multiples times, this would 
> lead to data being discarded at some level and regenerated at a lower level. 
> But that does not intrinsically make it a "problem" that needs solving.
> 
> > > >  - 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"*.
> 
> I can´t agree.
> 
> > > - 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).
> 
> That is complete nonsense. The cache filter is the second most upstream 
> element in the chain after the access. The whole point is to provide caching 
> for everything downstream, rather than only the demuxer.
> 
> If we moved the cache further downstream, and in particular after any 
> decompression filters, the prefetch filter would fail to fill its original 
> design role: to keep draining network buffers to solve congestion problems 
> with high bit rate network streams.
> 
> > > - 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`.
> 
> It is actually very common for demuxers to skip all or part of what they poke. 
> And this affects all stream types, not just block-based ones. Also AFAIR, 
> there was already a different incarnation of the same optimization before.
> 
> > 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.
> 
> I think I have made the arguments patently clear at this point.
> 
> And depending on how you look at it, the patch either fixes a nonexistent 
> problem or fixes a real problem in a hopelessly narrow subset of 
> circumstances.
> 
> -- 
> 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/7bac352c/attachment.html>