[vlc-devel] [PATCH 6/9] preparser: add playlist_preparser_Cancel

[Thomas Guillem]

On Wed, Jun 1, 2016, at 19:38, Rémi Denis-Courmont wrote:
> On Wednesday 01 June 2016 18:46:33 Thomas Guillem wrote:
> > On Wed, Jun 1, 2016, at 16:25, Rémi Denis-Courmont wrote:
> > > Le 2016-06-01 13:24, Thomas Guillem a écrit :
> > > > ---
> > > > 
> > > >  src/playlist/preparser.c | 23 +++++++++++++++++++++++
> > > >  src/playlist/preparser.h |  8 ++++++++
> > > >  2 files changed, 31 insertions(+)
> > > > 
> > > > diff --git a/src/playlist/preparser.c b/src/playlist/preparser.c
> > > > index a5751c5..13d5d3c 100644
> > > > --- a/src/playlist/preparser.c
> > > > +++ b/src/playlist/preparser.c
> > > > @@ -116,6 +116,29 @@ void playlist_preparser_fetcher_Push(
> > > > playlist_preparser_t *p_preparser,
> > > > 
> > > >          playlist_fetcher_Push( p_preparser->p_fetcher, p_item,
> > > > 
> > > > i_options );
> > > > 
> > > >  }
> > > > 
> > > > +void playlist_preparser_Cancel( playlist_preparser_t *p_preparser,
> > > > +                                input_item_t *p_item )
> > > > +{
> > > > +    vlc_mutex_lock( &p_preparser->lock );
> > > > +    /* Remove the item from the queue (if present) */
> > > > +    for( unsigned int i = 0; i < p_preparser->i_waiting; ++i )
> > > > +    {
> > > > +        preparser_entry_t *p_entry = p_preparser->pp_waiting[0];
> > > > +        if( p_entry->p_item == p_item )
> > > > +        {
> > > > +            vlc_gc_decref( p_entry->p_item );
> > > > +            free( p_entry );
> > > > +            REMOVE_ELEM( p_preparser->pp_waiting,
> > > > p_preparser->i_waiting, i );
> > > > +            break;
> > > > +        }
> > > > +    }
> > > > +    /* Stop the input_thread reading the item (if any) */
> > > > +    if( p_preparser->input != NULL
> > > > +     && input_GetItem( p_preparser->input ) == p_item )
> > > > +        input_Stop( p_preparser->input );
> > > > +    vlc_mutex_unlock( &p_preparser->lock );
> > > > +}
> > > > +
> > > 
> > > Items are reference counted, so you can have a situation where more
> > > than one something wants to preparse the same item. So, you can't just
> > > cancel it like that.
> > 
> > Ah yes, good catch.
> > I modified my patches: playlist_preparser_Push() can take a void *. This
> > void * can be used as a unique id to identity the caller of the request.
> > playlist_preparser_Cancel() cancel all requests done with this id.
> 
> I think it´s overkill and fear it will end up worse rather than better.
> 
> Of course, it´s tempting to cancel preparsing when the UI is no longer
> showing 
> the item. But then again, if you started preparsing, might as well finish
> it 
> for next time. All the more so considering that items in the playlist
> cannot 
> be truly deleted until the playlist itself is deleted (with the current
> crappy 
> design). You might end up starting, canceling, then starting again then 
> canceling again and so on. Better start, finish, and be done with it once
> and 
> for all.
> 
> It´s also quite a lot of state to retain and complexity on both side for, 
> AFAIK, little gain.

The gain is huge, especially for libvlc. If you parse a network file
that is stuck in network I/O, the parser will never receive any
stop/interrupt, so you won't be able to parse any more files (local or
network) from it.

> 
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel