[vlc-devel] [PATCH 2/7] playlist/preparser: refactor
Rémi Denis-Courmont
remi at remlab.net
Wed Mar 29 19:30:32 CEST 2017
Le perjantaina 24. maaliskuuta 2017, 3.28.29 EEST Filip Roséen a écrit :
> This refactoring should not only allow for easier maintenance as the
> code size has shrunk, it also implements a few advantages over the
> previous implementation:
>
> - playlist_preparser_Cancel is now optionally blocking if the
> referred to item is currently being preparsed (required in cases
> where another action would race with the preparser, such as
> playback (as preparsing and playing an entity at the same time can
> lead to duplicate items in the playlist).
That sounds very prone to deadlock and therefore a bad idea. (Though you could
argue that cancel itself is the bad idea.)
> - the congestion in terms of interacting with the preparser, and the
> preparsing itself, is lower. Meaning that we will finish a queue of
> items to preparse faster than with the old implementation.
>
> --
>
> Changes since previous submission:
>
> - Adjust to changes in terms of background-worker api
> ---
> src/playlist/preparser.c | 406
> +++++++++++++---------------------------------- 1 file changed, 109
> insertions(+), 297 deletions(-)
>
> diff --git a/src/playlist/preparser.c b/src/playlist/preparser.c
> index dd25284e81..c6cdab2b2c 100644
> --- a/src/playlist/preparser.c
> +++ b/src/playlist/preparser.c
> @@ -1,12 +1,9 @@
> /**************************************************************************
> *** - * preparser.c: Preparser thread.
> + * preparser.c
>
> ***************************************************************************
> ** - * Copyright © 1999-2009 VLC authors and VideoLAN
> + * Copyright © 2017-2017 VLC authors and VideoLAN
> * $Id$
> *
> - * Authors: Samuel Hocevar <sam at zoy.org>
> - * Clément Stenac <zorglub at videolan.org>
> - *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of the GNU Lesser General Public License as published by
> * the Free Software Foundation; either version 2.1 of the License, or @@
> -21,351 +18,166 @@
> * along with this program; if not, write to the Free Software Foundation,
> * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>
> ***************************************************************************
> **/ +
> #ifdef HAVE_CONFIG_H
> # include "config.h"
> #endif
>
> -#include <assert.h>
> -
> #include <vlc_common.h>
>
> -#include "fetcher.h"
> -#include "preparser.h"
> +#include "misc/background_worker.h"
> #include "input/input_interface.h"
> -
> -/**************************************************************************
> *** - * Structures/definitions
> -
> ***************************************************************************
> **/ -typedef struct preparser_entry_t preparser_entry_t;
> -
> -struct preparser_entry_t
> -{
> - input_item_t *p_item;
> - input_item_meta_request_option_t i_options;
> - void *id;
> - mtime_t timeout;
> -};
> +#include "input/input_internal.h"
> +#include "preparser.h"
> +#include "fetcher.h"
>
> struct playlist_preparser_t
> {
> - vlc_object_t *object;
> - playlist_fetcher_t *p_fetcher;
> - mtime_t default_timeout;
> -
> - void *input_id;
> - enum {
> - INPUT_RUNNING,
> - INPUT_STOPPED,
> - INPUT_CANCELED,
> - } input_state;
> -
> - vlc_mutex_t lock;
> - vlc_cond_t wait;
> - vlc_cond_t thread_wait;
> - bool b_live;
> - preparser_entry_t **pp_waiting;
> - size_t i_waiting;
> + vlc_object_t* owner;
> + playlist_fetcher_t* fetcher;
> + struct background_worker* worker;
> };
>
> -static void *Thread( void * );
> -
> -/**************************************************************************
> *** - * Public functions
> -
> ***************************************************************************
> **/ -playlist_preparser_t *playlist_preparser_New( vlc_object_t *parent )
> +static int InputEvent( vlc_object_t* obj, const char* varname,
> + vlc_value_t old, vlc_value_t cur, void* worker )
> {
> - playlist_preparser_t *p_preparser = malloc( sizeof(*p_preparser) );
> - if( !p_preparser )
> - return NULL;
> + VLC_UNUSED( obj ); VLC_UNUSED( varname ); VLC_UNUSED( old );
>
> - p_preparser->input_id = NULL;
> - p_preparser->input_state = INPUT_RUNNING;
> - p_preparser->object = parent;
> - p_preparser->default_timeout = var_InheritInteger( parent,
> "preparse-timeout" ); - p_preparser->p_fetcher = playlist_fetcher_New(
> parent );
> - if( unlikely(p_preparser->p_fetcher == NULL) )
> - msg_Err( parent, "cannot create fetcher" );
> -
> - vlc_mutex_init( &p_preparser->lock );
> - vlc_cond_init( &p_preparser->wait );
> - vlc_cond_init( &p_preparser->thread_wait );
> - p_preparser->b_live = false;
> - p_preparser->i_waiting = 0;
> - p_preparser->pp_waiting = NULL;
> -
> - return p_preparser;
> -}
> -
> -void playlist_preparser_Push( playlist_preparser_t *p_preparser,
> input_item_t *p_item, -
> input_item_meta_request_option_t i_options, -
> int timeout, void *id )
> -{
> - preparser_entry_t *p_entry = malloc( sizeof(preparser_entry_t) );
> -
> - if ( !p_entry )
> - return;
> - p_entry->p_item = p_item;
> - p_entry->i_options = i_options;
> - p_entry->id = id;
> - p_entry->timeout = (timeout < 0 ? p_preparser->default_timeout :
> timeout) * 1000; - vlc_gc_incref( p_entry->p_item );
> -
> - vlc_mutex_lock( &p_preparser->lock );
> - INSERT_ELEM( p_preparser->pp_waiting, p_preparser->i_waiting,
> - p_preparser->i_waiting, p_entry );
> - if( !p_preparser->b_live )
> - {
> - if( vlc_clone_detach( NULL, Thread, p_preparser,
> - VLC_THREAD_PRIORITY_LOW ) )
> - msg_Warn( p_preparser->object, "cannot spawn pre-parser thread"
> ); - else
> - p_preparser->b_live = true;
> - }
> - vlc_mutex_unlock( &p_preparser->lock );
> -}
> + if( cur.i_int == INPUT_EVENT_DEAD )
> + background_worker_RequestProbe( worker );
It might work, but it is probably either a misnomer, or a hack.
>
> -void playlist_preparser_fetcher_Push( playlist_preparser_t *p_preparser,
> - input_item_t *p_item, input_item_meta_request_option_t
> i_options ) -{
> - if( p_preparser->p_fetcher != NULL )
> - playlist_fetcher_Push( p_preparser->p_fetcher, p_item, i_options );
> + return VLC_SUCCESS;
> }
>
> -void playlist_preparser_Cancel( playlist_preparser_t *p_preparser, void *id
> ) +static int PreparserOpenInput( void* preparser_, void* item_, void** out
> ) {
> - assert( id != NULL );
> - vlc_mutex_lock( &p_preparser->lock );
> + playlist_preparser_t* preparser = preparser_;
>
> - /* Remove entries that match with the id */
> - for( int i = p_preparser->i_waiting - 1; i >= 0; --i )
> + input_thread_t* input = input_CreatePreparser( preparser->owner, item_
> ); + if( !input )
> {
> - preparser_entry_t *p_entry = p_preparser->pp_waiting[i];
> - if( p_entry->id == id )
> - {
> - vlc_gc_decref( p_entry->p_item );
> - free( p_entry );
> - REMOVE_ELEM( p_preparser->pp_waiting, p_preparser->i_waiting, i
> ); - }
> + input_item_SignalPreparseEnded( item_, ITEM_PREPARSE_FAILED );
> + return VLC_EGENERIC;
> }
>
> - /* Stop the input_thread reading the item (if any) */
> - if( p_preparser->input_id == id )
> + var_AddCallback( input, "intf-event", InputEvent, preparser->worker );
> + if( input_Start( input ) )
> {
> - p_preparser->input_state = INPUT_CANCELED;
> - vlc_cond_signal( &p_preparser->thread_wait );
> + input_Close( input );
> + var_DelCallback( input, "intf-event", InputEvent, preparser->worker
> ); + input_item_SignalPreparseEnded( item_, ITEM_PREPARSE_FAILED );
> + return VLC_EGENERIC;
> }
> - vlc_mutex_unlock( &p_preparser->lock );
> +
> + *out = input;
> + return VLC_SUCCESS;
> }
>
> -void playlist_preparser_Delete( playlist_preparser_t *p_preparser )
> +static int PreparserProbeInput( void* preparser_, void* input_ )
> {
> - vlc_mutex_lock( &p_preparser->lock );
> - /* Remove pending item to speed up preparser thread exit */
> - while( p_preparser->i_waiting > 0 )
> - {
> - preparser_entry_t *p_entry = p_preparser->pp_waiting[0];
> - vlc_gc_decref( p_entry->p_item );
> - free( p_entry );
> - REMOVE_ELEM( p_preparser->pp_waiting, p_preparser->i_waiting, 0 );
> - }
> + return input_GetState( input_ ) == END_S;
> + VLC_UNUSED( preparser_ );
> +}
>
> - p_preparser->input_state = INPUT_CANCELED;
> - vlc_cond_signal( &p_preparser->thread_wait );
> +static void PreparserCloseInput( void* preparser_, void* input_ )
> +{
> + playlist_preparser_t* preparser = preparser_;
> + input_thread_t* input = input_;
> + input_item_t* item = input_priv(input)->p_item;
>
> - while( p_preparser->b_live )
> - vlc_cond_wait( &p_preparser->wait, &p_preparser->lock );
> - vlc_mutex_unlock( &p_preparser->lock );
> + var_DelCallback( input, "intf-event", InputEvent, preparser->worker );
>
> - /* Destroy the item preparser */
> - vlc_cond_destroy( &p_preparser->thread_wait );
> - vlc_cond_destroy( &p_preparser->wait );
> - vlc_mutex_destroy( &p_preparser->lock );
> + int status = input_GetState( input ) == END_S
> + ? ITEM_PREPARSE_DONE : ITEM_PREPARSE_TIMEOUT;
>
> - if( p_preparser->p_fetcher != NULL )
> - playlist_fetcher_Delete( p_preparser->p_fetcher );
> - free( p_preparser );
> + input_Stop( input );
> + input_Close( input );
> +
> + if( preparser->fetcher )
> + playlist_fetcher_Push( preparser->fetcher, item, 0 );
> +
> + input_item_SetPreparsed( item, true );
> + input_item_SignalPreparseEnded( item, status );
> }
>
> -/**************************************************************************
> *** - * Privates functions
> -
> ***************************************************************************
> **/ +static void InputItemRelease( void* item ) { input_item_Release( item
> ); } +static void InputItemHold( void* item ) { input_item_Hold( item ); }
>
> -static int InputEvent( vlc_object_t *obj, const char *varname,
> - vlc_value_t old, vlc_value_t cur, void *data )
> +playlist_preparser_t* playlist_preparser_New( vlc_object_t *parent )
> {
> - playlist_preparser_t *preparser = data;
> - int event = cur.i_int;
> + playlist_preparser_t* preparser = malloc( sizeof *preparser );
>
> - if( event == INPUT_EVENT_DEAD )
> - {
> - vlc_mutex_lock( &preparser->lock );
> + struct background_worker_config conf = {
> + .default_timeout = var_InheritInteger( parent, "preparse-timeout"
> ), + .pf_start = PreparserOpenInput,
> + .pf_probe = PreparserProbeInput,
> + .pf_stop = PreparserCloseInput,
> + .pf_release = InputItemRelease,
> + .pf_hold = InputItemHold };
>
> - preparser->input_state = INPUT_STOPPED;
> - vlc_cond_signal( &preparser->thread_wait );
>
> - vlc_mutex_unlock( &preparser->lock );
> + if( likely( preparser ) )
> + preparser->worker = background_worker_New( preparser, &conf );
> +
> + if( unlikely( !preparser || !preparser->worker ) )
> + {
> + free( preparser );
> + return NULL;
> }
>
> - (void) obj; (void) varname; (void) old;
> - return VLC_SUCCESS;
> + preparser->owner = parent;
> + preparser->fetcher = playlist_fetcher_New( parent );
> +
> + if( unlikely( !preparser->fetcher ) )
> + msg_Warn( parent, "unable to create art fetcher" );
> +
> + return preparser;
> }
>
> -/**
> - * This function preparses an item when needed.
> - */
> -static void Preparse( playlist_preparser_t *preparser,
> - preparser_entry_t *p_entry )
> +void playlist_preparser_Push( playlist_preparser_t *preparser,
> + input_item_t *item, input_item_meta_request_option_t i_options,
> + int timeout, void *id )
> {
> - input_item_t *p_item = p_entry->p_item;
> -
> - vlc_mutex_lock( &p_item->lock );
> - int i_type = p_item->i_type;
> - bool b_net = p_item->b_net;
> - vlc_mutex_unlock( &p_item->lock );
> -
> - bool b_preparse = false;
> - switch (i_type) {
> - case ITEM_TYPE_FILE:
> - case ITEM_TYPE_DIRECTORY:
> - case ITEM_TYPE_PLAYLIST:
> - case ITEM_TYPE_NODE:
> - if( !b_net || p_entry->i_options &
> META_REQUEST_OPTION_SCOPE_NETWORK ) - b_preparse = true;
> - break;
> - }
> + vlc_mutex_lock( &item->lock );
> + int i_type = item->i_type;
> + int b_net = item->b_net;
> + vlc_mutex_unlock( &item->lock );
>
> - /* Do not preparse if it is already done (like by playing it) */
> - if( b_preparse && !input_item_IsPreparsed( p_item ) )
> + switch( i_type )
> {
> - int status;
> - input_thread_t *input = input_CreatePreparser( preparser->object,
> p_item ); - if( input == NULL )
> - {
> - input_item_SignalPreparseEnded( p_item, ITEM_PREPARSE_FAILED );
> + case ITEM_TYPE_NODE:
> + case ITEM_TYPE_FILE:
> + case ITEM_TYPE_DIRECTORY:
> + case ITEM_TYPE_PLAYLIST:
> + if( !b_net || i_options & META_REQUEST_OPTION_SCOPE_NETWORK )
> + break;
> + default:
> + input_item_SignalPreparseEnded( item, ITEM_PREPARSE_SKIPPED );
> return;
> - }
> -
> - var_AddCallback( input, "intf-event", InputEvent, preparser );
> - if( input_Start( input ) == VLC_SUCCESS )
> - {
> - vlc_mutex_lock( &preparser->lock );
> -
> - if( p_entry->timeout > 0 )
> - {
> - mtime_t deadline = mdate() + p_entry->timeout;
> - while( preparser->input_state == INPUT_RUNNING )
> - {
> - if( vlc_cond_timedwait( &preparser->thread_wait,
> - &preparser->lock, deadline ) )
> - preparser->input_state = INPUT_CANCELED; /* timeout
> */ - }
> - }
> - else
> - {
> - while( preparser->input_state == INPUT_RUNNING )
> - vlc_cond_wait( &preparser->thread_wait,
> &preparser->lock ); - }
> - assert( preparser->input_state == INPUT_STOPPED
> - || preparser->input_state == INPUT_CANCELED );
> - status = preparser->input_state == INPUT_STOPPED ?
> - ITEM_PREPARSE_DONE : ITEM_PREPARSE_TIMEOUT;
> -
> - vlc_mutex_unlock( &preparser->lock );
> - }
> - else
> - status = ITEM_PREPARSE_FAILED;
> -
> - var_DelCallback( input, "intf-event", InputEvent, preparser );
> - if( status == ITEM_PREPARSE_TIMEOUT )
> - input_Stop( input );
> - input_Close( input );
> -
> - var_SetAddress( preparser->object, "item-change", p_item );
> - input_item_SetPreparsed( p_item, true );
> - input_item_SignalPreparseEnded( p_item, status );
> }
> - else
> - input_item_SignalPreparseEnded( p_item, ITEM_PREPARSE_SKIPPED );
> +
> + if( background_worker_Push( preparser->worker, item, id, timeout ) )
> + input_item_SignalPreparseEnded( item, ITEM_PREPARSE_FAILED );
> }
>
> -/**
> - * This function ask the fetcher object to fetch the art when needed
> - */
> -static void Art( playlist_preparser_t *p_preparser, input_item_t *p_item )
> +void playlist_preparser_fetcher_Push( playlist_preparser_t *preparser,
> + input_item_t *item, input_item_meta_request_option_t options )
> {
> - vlc_object_t *obj = p_preparser->object;
> - playlist_fetcher_t *p_fetcher = p_preparser->p_fetcher;
> -
> - bool b_fetch = false;
> - /* If we haven't retrieved enough meta, add to secondary queue
> - * which will run the "meta fetchers".
> - * This only checks for meta, not for art
> - * \todo don't do this for things we won't get meta for, like vids
> - */
> -
> - vlc_mutex_lock( &p_item->lock );
> - if( p_item->p_meta )
> - {
> - const char *psz_arturl = vlc_meta_Get( p_item->p_meta,
> vlc_meta_ArtworkURL ); - const char *psz_name = vlc_meta_Get(
> p_item->p_meta, vlc_meta_Title ); -
> - if( !psz_arturl || ( strncasecmp( psz_arturl, "file://", 7 ) &&
> - strncasecmp( psz_arturl, "attachment://", 13 )
> ) ) - {
> - msg_Dbg( obj, "meta ok for %s, need to fetch art",
> - psz_name ? psz_name : "(null)" );
> - b_fetch = true;
> - }
> - else
> - {
> - msg_Dbg( obj, "no fetch required for %s (art currently %s)",
> - psz_name ? psz_name : "(null)",
> - psz_arturl ? psz_arturl : "(null)" );
> - }
> - }
> - vlc_mutex_unlock( &p_item->lock );
> + if( preparser->fetcher )
> + playlist_fetcher_Push( preparser->fetcher, item, options );
> +}
>
> - if( b_fetch && p_fetcher )
> - playlist_fetcher_Push( p_fetcher, p_item, 0 );
> +void playlist_preparser_Cancel( playlist_preparser_t *preparser, void *id )
> +{
> + background_worker_Cancel( preparser->worker, id );
> }
>
> -/**
> - * This function does the preparsing and issues the art fetching requests
> - */
> -static void *Thread( void *data )
> +void playlist_preparser_Delete( playlist_preparser_t *preparser )
> {
> - playlist_preparser_t *p_preparser = data;
> + background_worker_Delete( preparser->worker );
>
> - for( ;; )
> - {
> - preparser_entry_t *p_entry;
> -
> - vlc_mutex_lock( &p_preparser->lock );
> - /* */
> - p_preparser->input_state = INPUT_RUNNING;
> - if( p_preparser->i_waiting > 0 )
> - {
> - p_entry = p_preparser->pp_waiting[0];
> - p_preparser->input_id = p_entry->id;
> - REMOVE_ELEM( p_preparser->pp_waiting, p_preparser->i_waiting, 0
> ); - }
> - else
> - {
> - p_preparser->b_live = false;
> - p_preparser->input_id = NULL;
> - vlc_cond_signal( &p_preparser->wait );
> - vlc_mutex_unlock( &p_preparser->lock );
> - break;
> - }
> - vlc_mutex_unlock( &p_preparser->lock );
> - assert( p_entry );
> -
> - Preparse( p_preparser, p_entry );
> -
> - Art( p_preparser, p_entry->p_item );
> - vlc_gc_decref( p_entry->p_item );
> - free( p_entry );
> - }
> - return NULL;
> -}
> + if( preparser->fetcher )
> + playlist_fetcher_Delete( preparser->fetcher );
>
> + free( preparser );
> +}
--
雷米‧德尼-库尔蒙
https://www.remlab.net/
More information about the vlc-devel
mailing list