[vlc-devel] [PATCH 2/7] playlist/preparser: refactor

Filip Roséen filip at atch.se
Wed Mar 22 17:58:42 CET 2017


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).

 - 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.
---
 src/playlist/misc/background_worker.c |   2 +-
 src/playlist/preparser.c              | 402 +++++++++-------------------------
 2 files changed, 105 insertions(+), 299 deletions(-)

diff --git a/src/playlist/misc/background_worker.c b/src/playlist/misc/background_worker.c
index 0c6041e6ef..61eaddf9a2 100644
--- a/src/playlist/misc/background_worker.c
+++ b/src/playlist/misc/background_worker.c
@@ -79,7 +79,7 @@ static void* Thread( void* data )
                                  priv->current.deadline );
         }
 
-        priv->current.item = NULL;
+        FREENULL( priv->current.item );
         priv->current.handle = NULL;
     }
 
diff --git a/src/playlist/preparser.c b/src/playlist/preparser.c
index dd25284e81..acba6f84c6 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,160 @@
  * 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;
-}
+    if( cur.i_int == INPUT_EVENT_DEAD )
+        background_worker_Signal( worker );
 
-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 );
-}
-
-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_ );
+}
+
+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;
+
+    var_DelCallback( input, "intf-event", InputEvent, &preparser->worker );
 
-    p_preparser->input_state = INPUT_CANCELED;
-    vlc_cond_signal( &p_preparser->thread_wait );
+    int status = input_GetState( input ) == END_S
+        ? ITEM_PREPARSE_DONE : ITEM_PREPARSE_TIMEOUT;
 
-    while( p_preparser->b_live )
-        vlc_cond_wait( &p_preparser->wait, &p_preparser->lock );
-    vlc_mutex_unlock( &p_preparser->lock );
+    input_Stop( input );
+    input_Close( input );
 
-    /* Destroy the item preparser */
-    vlc_cond_destroy( &p_preparser->thread_wait );
-    vlc_cond_destroy( &p_preparser->wait );
-    vlc_mutex_destroy( &p_preparser->lock );
+    if( preparser->fetcher )
+        playlist_fetcher_Push( preparser->fetcher, item, 0 );
 
-    if( p_preparser->p_fetcher != NULL )
-        playlist_fetcher_Delete( p_preparser->p_fetcher );
-    free( p_preparser );
+    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 );
+    if( unlikely( !preparser ) )
+        return NULL;
 
-        preparser->input_state = INPUT_STOPPED;
-        vlc_cond_signal( &preparser->thread_wait );
+    preparser->owner = parent;
+    preparser->fetcher = playlist_fetcher_New( parent );
 
-        vlc_mutex_unlock( &preparser->lock );
-    }
+    if( unlikely( !preparser->fetcher ) )
+        msg_Warn( parent, "unable to create art fetcher" );
 
-    (void) obj; (void) varname; (void) old;
-    return VLC_SUCCESS;
+    preparser->worker.pf_start = PreparserOpenInput;
+    preparser->worker.pf_probe = PreparserProbeInput;
+    preparser->worker.pf_stop = PreparserCloseInput;
+    preparser->worker.pf_release = InputItemRelease;
+    preparser->worker.pf_hold = InputItemHold;
+
+    background_worker_Init( &preparser->worker, preparser,
+          var_InheritInteger( parent, "preparse-timeout" ) );
+
+    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_Clean( &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 );
+}
-- 
2.12.0


More information about the vlc-devel mailing list