[vlc-devel] [PATCH 1/6] input/stream: add stream_extractor_AttachedParsed

[Filip Roséen]

Hi Hugo,

On 2017-03-17 11:08, Hugo Beauzée-Luyssen wrote:

> On Fri, Mar 17, 2017, at 03:22 AM, Filip Roséen wrote:
> > +int
> > +stream_extractor_AttachParsed( stream_t** source, char const* data,
> > +                               char const** out_extra )
> > +{
> > +    vlc_array_t identifiers;
> > +
> > +    if( mrl_FragmentSplit( &identifiers, out_extra, data ) )
> > +        return VLC_EGENERIC;
> > +
> > +    while( vlc_array_count( &identifiers ) )
> > +    {
> > +        char* id = vlc_array_item_at_index( &identifiers, 0 );
> > +
> > +        if( vlc_stream_extractor_Attach( source, id, NULL ) )
> > +            break;
> > +
> > +        vlc_array_remove( &identifiers, 0 );
> > +        free( id );
> > +    }
> > +
> > +    size_t remaining = vlc_array_count( &identifiers );
> > +
> > +    for( size_t i = 0; i < remaining; ++i )
> > +        free( vlc_array_item_at_index( &identifiers, i ) );
> > +    vlc_array_clear( &identifiers );
> > +
> > +    if( remaining == 0 )
> > +    {
> > +        vlc_stream_extractor_Attach( source, NULL, NULL );
> > +        return VLC_SUCCESS;
> > +    }
> > +
> > +    return remaining == 0 ? VLC_SUCCESS : VLC_EGENERIC;
> > +}
> 
> Couldn't this be written in a way that has no vlc_array_remove and a
> single unconditional vlc_array_clean?

Sure, it could - I simply wrote it the way proposed as shrinking and
moving the underlying storage is not exactly expensive in the path
taken, nor would this function normally be called with a trillion
elements in `identifiers`.

I found it easier to read using `while` instead of `for`, but please
see attached patch for an updated one (that also removes a trailing
if-branch that should not be part of the function, the removal was
accidentally rebased upon a later commit instead if this one).

> You should be able to guess if some items weren't processed if you
> index is < vlc_array_count. This would prevent quite a bunch of
> reallocs/memmove

Correct, but there's strictly speaking no need to guess `;-)`.

Thanks,\
Filip Roséen

> > +
> >  char*
> >  vlc_stream_extractor_CreateMRL( stream_directory_t* directory,
> >                                  char const* subentry )
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170317/78e838ea/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-input-stream-add-stream_extractor_AttachedParsed.patch
Type: text/x-diff
Size: 3674 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170317/78e838ea/attachment-0001.patch>