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

[Rémi Denis-Courmont]

Le maanantaina 7. marraskuuta 2016, 18.36.46 EET Filip Roséen a écrit :
> 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.

No, you can´t: The offset needs to be stored somewhere if you peek more than 
one line. Indeed, the current code has it, your patch and it, and my wrapper 
has it.

You also need to store some state associated with character encoding 
conversion one way or another. Or from a high level point of view, peeking 
lines should not affect the underlying stream state for later demuxers in the 
probing sequence.

-- 
Rémi Denis-Courmont
http://www.remlab.net/