<p dir="ltr">Hello,</p>
<p dir="ltr">I don't follow. The "penalty" of block-based access to stream-based demux is a memory copy. It is intrinsic.</p>
<p dir="ltr">Furthermore, the micro-optimization in this patch does not remove or add any memory copy as such.</p>
<div class="gmail_extra"><br><div class="gmail_quote">Le 24 oct. 2016 2:21 PM, Filip Roséen <filip@atch.se> a écrit :<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


  
  
  
  
  

<div>
<p>Hi Rémi,</p>
<p>I honestly feel like you are misunderstanding what I am trying to convay, if that is me being vague in my explanation or if we are just talking different languages - I do not know.</p>
<p>Given that we have a transparent proxy going from block-based reading to what most people associate with “stream”, it is very unfortunate that block-based accessors suffer a penalty because of that.</p>
<p>The reading on the “output” end should not have to worry about the resource which it is reading data from, and should certainly /not/ care whether the input-side is a <em>block-</em> or <em>stream-based</em> stream.</p>
<p>In my opinion it does not make any sense for us to copy the same data over and over, no matter if one issues a seek or does a proper read (skip) into a <code>NULL</code>-buffer.</p>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Le maanantaina 24. lokakuuta 2016, 10.38.03 EEST Filip Roséen a écrit :</code></pre>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Hi Remi

 On 2016-10-24 11:33, Rémi Denis-Courmont wrote:</code></pre>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Le maanantaina 24. lokakuuta 2016, 2.13.11 EEST Filip Roséen a écrit :</code></pre>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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).</code></pre>
</blockquote>
<pre><code> 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.</code></pre>
</blockquote>
<pre><code> 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..</code></pre>
</blockquote>
<pre><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.</code></pre>
</blockquote>
<p>This is besides the point.</p>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> and I think it make sense not to re-read data
 (just as we have the functionality in place for peeking).</code></pre>
</blockquote>
<pre><code> Yes. That´s what VLC and operating systems have caches for.</code></pre>
</blockquote>
<p>VLC has a conditional cache, and when these cache modules not apply, performance suck; that’s it.</p>
<p>And on that note, the current cache mechanisms are applied too early in the chain - given that <em>stream_filter</em> (applied after the <em>accessor</em> is created) cannot take advantage of the cache-modules, reading from a block-based <em>stream_filter</em> suffers from the “there is no cache”-flaw, no matter what.</p>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> One could even merge the peeking and block "cache" in
 `src/input/stream.c`, making the implementation even less complicated
 than it is now.</code></pre>
</blockquote>
<pre><code> 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.</code></pre>
</blockquote>
<p>Support for peeking exists, the support of block in this regard is certainly not as good as it could be (and given that you just said that such complexity is not supposed to be there, you inherently say that support for not copying the same data over and over is not there).</p>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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.</code></pre>
</blockquote>
<pre><code> 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).</code></pre>
</blockquote>
<pre><code> 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.
 </code></pre>
</blockquote>
<p>I am well aware of the above, but I honestly feel like bringing up such explanation of how things work is drawing attention away from the real issues:</p>
<ul><li><p>We are copying data over and over if you happen to seek within what is the same block for a <em>block-based</em> stream.</p></li><li><p>Given that the <em>reader-side</em> cannot know the size of the block coming from the <em>input-side</em> (unless it is reading blocks):</p></li><li><p>How can it know that a <em>read-skip</em> is better than seek (given that the block size can be arbitrary)?</p></li><li><p>Should we force implementations not to issue seek-requests at all, and just rely on skipping?</p></li><li><p>Why not make the skipping and seeking be based on the same implementation? It is semantically the same thing.</p></li></ul>
<p>This is not a matter of adding complexity, and as I already said rewriting the relevant part of <code>src/input/stream.c</code> so that the functionality is shared would even <em>simplify</em> the implementation (not the opposite).</p>
<p>I do not know what else to add, I am honestly quite baffled about the negativity received;</p>
<ul><li>what do you propose to fix the problem, or;</li><li>are you saying that there is no problem to be fixed?</li></ul>
<p>Best Regards,<br />
Filip</p>
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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.</code></pre>
</blockquote>
</blockquote>
</div>

</blockquote></div><br></div>