[vlc-devel] [PATCH 1/2] demux/subtitles: add new implementation for line-peeking

[Filip Roséen]

Hi Remi,

On 2016-11-07 19:08, Rémi Denis-Courmont wrote:

> Well OK, that invalidates my comments. But it still does not seem right.

I do however agree that it is a hack written for a very specific
use-case.
 
> At best, you end up with quadratic complexity processing the same lines over
> and over again. But IMHO the worst lies in second guessing
> vlc_stream_ReadLine().

The above sounds worse than it would actually be in practice, you will
potentially "process" the initial line several times, as well as
partial data of a line that is longer than the peek size increase
(`2048` in the supplied patch).

In practice this should not happen too often.

> If you want to peek lines, you should rather use a proper stream wrapper than
> create memory streams over and over again, e.g. (completely untested):

Indeed, I was however hesitant to add such functionality in the core
due to the currently limited request/workarounds present for this
functionality.

I would be happy to add support for this in the core (so that everyone
can take part of it), but I see no point of having an explicit
*wrapper*; one can implement the functionality directly within
`src/input/stream.c` without having to modify any part of the internal
structure.

Do you think it is a good idea to implement the below listed functions
in the relevant *TU* (and exposing them to the world)?

  - `vlc_stream_PeekReadLine`
  - `vlc_stream_PeekRead` (for symmetry purposes)

Both functions would behave like their *non-peeking* counterpart, but
not advance the read-offset within the stream. Not sure if the names
are descriptive enough, but hopefully their purpose is clear enough to
at least discuss the possibility of adding such functions.

Let me know if you find the above applicable, and I can submit a patch
for it later this evening.

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161107/23d0cae6/attachment.html>