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

[Filip Roséen]

Hi Remi,

On 2016-11-06 21:29, Rémi Denis-Courmont wrote:

> Le sunnuntaina 6. marraskuuta 2016, 17.58.44 EET Filip Roséen a écrit :
> > This added function properly handles streams where an initial BOM is
> > used to state the encoding used, effectively making sure that we do
> > not forget about the encoding of the stream while peeking lines.
> > ---
> >  modules/demux/subtitle_helper.h | 75
> > +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+)
> > 
> > diff --git a/modules/demux/subtitle_helper.h
> > b/modules/demux/subtitle_helper.h index b5f18da..fc71c12 100644
> > --- a/modules/demux/subtitle_helper.h
> > +++ b/modules/demux/subtitle_helper.h
> > @@ -18,6 +18,9 @@
> >   * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> >  
> > ***************************************************************************
> > **/
> > 
> > +#include <vlc_common.h>
> > +#include <vlc_stream.h>
> > +
> >  inline static char * peek_Readline( stream_t *p_demuxstream, uint64_t
> > *pi_offset ) {
> >      uint8_t *p_peek;
> > @@ -43,3 +46,75 @@ inline static char * peek_Readline( stream_t
> > *p_demuxstream, uint64_t *pi_offset
> > 
> >      return psz_line;
> >  }
> > +
> > +/**
> > + * Peek next line of stream
> > + *
> > + * This function returns the next available line in \p p_source
> > + * without modifying the read-offset of the stream (meaning that it
> > + * effectivelly it behaves like vlc_stream_Peek, but for lines).
> > + *
> > + * \param source The stream for which line-peeking shall be made
> > + * \param peekstate shall be NULL on first invocation, and `*peekstate`
> > + *                  will be updated to reflect the internal state of the
> > + *                  peeking.
> > + *
> > + * \warning if `*peekstate` is not NULL, it shall be released by the
> > + *          callee when no more data is desired to be peeked (using
> > + *          \link vlc_stream_Delete).
> > + *
> > + * \return pointer to the next line (if any), must be freed by the
> > + *         callee.
> > + **/
> > +inline static char * subtitle_PeekLine( stream_t *source, stream_t
> > **peekstate ) +{
> > +    for( uint64_t offset = 0, size = 0;; )
> > +    {
> > +        char* line = NULL;
> > +
> > +        if( *peekstate )
> > +        {
> > +            if( vlc_stream_GetSize( *peekstate, &size ) )
> > +                return NULL;
> 
> The whole point of the faulty commit was to support probing subtitles from 
> nonseekable streams. Nonseekable streams typically do not have a known size, 
> so this is an extremely dubious requirement.
> 
> In other words, reverting the faulty commit would be a lot easier.

`*peekstate` refers to a `stream_t` created by `vlc_stream_MemoryNew`,
as such it is safe to do the operations within `subtitle_PeekLine` (it
is not related to seekability of the demuxer stream).

> > +
> > +            offset = vlc_stream_Tell( *peekstate );
> > +            line   = vlc_stream_ReadLine( *peekstate );
> 
> Likewise, you can´t do that here; this is missing the point.

See previous comment.

> > +
> > +            if( line && vlc_stream_Tell( *peekstate ) < size )
> > +                return line;
> > +        }
> > +
> > +        uint8_t const* data;
> > +        ssize_t peeked = vlc_stream_Peek( source, &data, size + 2048 );
> > +
> > +        if( peeked > 0 && (uint64_t)peeked <= size )
> > +            return line;
> > +        else
> > +            free( line );
> > +
> > +        if( peeked < 0 )
> > +            return NULL;
> > +
> > +        if( *peekstate )
> > +        {
> > +            vlc_stream_Delete( *peekstate );
> > +            *peekstate = NULL;
> > +        }
> > +
> > +        *peekstate = vlc_stream_MemoryNew( source, (uint8_t*)data,
> > +                                           peeked, true );
> > +        if( unlikely( !*peekstate ) )
> > +            return NULL;
> > +
> > +        if( offset != 0 )
> > +        {
> > +            /* read potential BOM */
> > +            free( vlc_stream_ReadLine( *peekstate ) );
> > +
> > +            if( vlc_stream_Seek( *peekstate, offset ) )
> > +                return NULL;
> > +        }
> > +    }
> > +
> > +    vlc_assert_unreachable();
> > +}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161106/301026da/attachment.html>