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

[Filip Roséen]

Hi again,

On 2016-10-24 17:57, Rémi Denis-Courmont wrote:

> Le maanantaina 24. lokakuuta 2016, 16.27.05 EEST Filip Roséen a écrit :
> > I am talking about the *memory copy/write* that inherently must take
> > place in order for a block-based content *"generator"* to create a
> > `block_t` that is then living inside `src/input/stream.c`.
> > 
> >     [ block access ] -> [ block cache ] -> [core] -> [ stream reader ]
> >             ^- A                ^- B                       ^- C
> >     /**
> >      * A + B is somewhat redundant to the example, we could have left
> >      * out B and have the same behavior, but it is added to better
> >      * reflect how the previous discussion.
> >      *
> >      * One should however keep in mind that there might be any number
> >      * of stream_filters between "[core]" and "[stream reader]", and
> >      * if these are block-based things get worse.
> >      **/
> > 
> >  In the above `B` is simply a cache that will cache all the blocks
> >  that are generated by `A`, the `[core]` is the implementation within
> >  `src/input/stream.c`, and `C` is something reading using
> >  `vlc_stream_Read`.
> > 
> >  Imagine we are att offset zero, and `B` generates blocks `1 MB` in
> >  size, a block having `p_block->i_buffer = 1024*1024` would end up in
> >  `[core`].
> 
>  There is zero overhead compared to stream-based access. In both cases, we 
> seek the underlying stream, and read the data. Also in case of caching, if 
> buffers allow, in both cases, we just move the cursor within the cache.

I can only refer to what I wrote about the cache in previous replies.
 
> >  If `C` wants to read `256` bytes at offset zero, and then `256 bytes`
> >  at offset `524 288` it would (probably) do something as the below
> >  (even though it could use skipping, but it has no way of knowing that
> >  `B` has `N` as "chunk size"):
> > 
> >      vlc_stream_Read( src, buf_1, 256 ); /* (1) */
> >      vlc_stream_Seek( src, 524288 );     /* (2) */
> >      vlc_stream_Read( src, buf_2, 256 ); /* (3) */
> > 
> > Given that we currently, unconditionally, invalidate read blocks in
> > `[core]` during `vlc_stream_Seek`, at `(3)` we would have to ask `B`
> > to create a new block which the `[core]` transparently will use to
> > feed `C`.
> 
> And the exact same thing happens with a stream-based access. There is 
> absolutely no block-specific overhead here.

 - We do not ask *stream-based* accessors to give us `N` bytes of
   data, where `N` is a decision made by the *accessor* itself, in
   order for us to read a value that is not equivalent to `N`.

   If the *block-based* *accessor* can only read on even alignments of
   `X`, it will have to re-read data suitable for that alignment.

 - We ask streams exposing `pf_read` for `M` bytes of data, and they
   will try to fulfill that request.

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`.
> 
> > In other words, `B` will "regenerate" contents that was already
> > available at `(1)`, and for *accessors/stream_filters* that can only
> > fetch chunks of `N` bytes of data upstream, it would have to fetch `N`
> > bytes, even though reading stream is only interested in a smaller
> > subset.
> 
> That is not a memory copy.

It is inherently reading data from one place in order to store it in a
block, if you want to call that a *memory copy* or simply *writing
data into a block*, it does not change the fact that we create a new
block for the `[core]` to read.

> If B performs memory copy internally, then it should probably be
> stream-based in the first place. And regardless, that´s a problem of
> the access implementation, not the core.

 - So you are saying that an *accessor* that works with a remote entity
   that is block based should do the same logic that we could have had
   in the core; reading blocks, storing them, and then expose contents
   through `pf_read`?

If that is what you are saying, then we need to implement a fix for
every block-based *accessor/stream_filter* (legacy and future ones).

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?

Please keep in mind what I wrote about the cache-modules being too
early in the stream-chain, and how the problem simply shifts from one
place to another if one decides to move it.

> -- 
> 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/67d31c61/attachment.html>