[vlc-devel] [vlc-commits] variables: use linked listed for callbacks

Zhao Zhili quinkblack at foxmail.com
Wed Apr 18 14:30:51 CEST 2018



On 2018年04月18日 15:28, Thomas Guillem wrote:
> Hello,
>
> On Tue, Apr 17, 2018, at 21:35, Rémi Denis-Courmont wrote:
>> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Tue Apr
>> 17 21:35:08 2018 +0300| [c3e4633e1687ec1f0e4a41b99e8eef8797d83abe] |
>> committer: Rémi Denis-Courmont
>>
>> variables: use linked listed for callbacks
>>
>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=c3e4633e1687ec1f0e4a41b99e8eef8797d83abe
>> ---
>>
>>   src/misc/variables.c | 144 ++++++++++++++++++++++++++-------------------------
>>   1 file changed, 73 insertions(+), 71 deletions(-)
>>
>> diff --git a/src/misc/variables.c b/src/misc/variables.c
>> index 2426215954..c1ebd64302 100644
>> --- a/src/misc/variables.c
>> +++ b/src/misc/variables.c
>> @@ -45,6 +45,7 @@
>>   
>>   typedef struct callback_entry_t
>>   {
>> +    struct callback_entry_t *next;
>>       union
>>       {
>>           vlc_callback_t       pf_value_callback;
>> @@ -61,12 +62,6 @@ typedef struct variable_ops_t
>>       void (*pf_free) ( vlc_value_t * );
>>   } variable_ops_t;
>>   
>> -typedef struct callback_table_t
>> -{
>> -    int                i_entries;
>> -    callback_entry_t * p_entries;
>> -} callback_table_t;
>> -
>>   /**
>>    * The structure describing a variable.
>>    * \note vlc_value_t is the common union for variable values
>> @@ -98,9 +93,9 @@ struct variable_t
>>       bool   b_incallback;
>>   
>>       /** Registered value callbacks */
>> -    callback_table_t    value_callbacks;
>> +    callback_entry_t    *value_callbacks;
>>       /** Registered list callbacks */
>> -    callback_table_t    list_callbacks;
>> +    callback_entry_t    *list_callbacks;
>>   };
>>   
>>   static int CmpBool( vlc_value_t v, vlc_value_t w )
>> @@ -176,7 +171,14 @@ static void Destroy( variable_t *p_var )
>>   
>>       free( p_var->psz_name );
>>       free( p_var->psz_text );
>> -    free( p_var->value_callbacks.p_entries );
>> +    while (unlikely(p_var->value_callbacks != NULL))
>> +    {
>> +        callback_entry_t *next = p_var->value_callbacks->next;
>> +
>> +        free(p_var->value_callbacks);
>> +        p_var->value_callbacks = next;
>> +    }
>> +    assert(p_var->list_callbacks == NULL);
>>       free( p_var );
>>   }
>>   
>> @@ -237,20 +239,22 @@ static void TriggerCallback(vlc_object_t *obj,
>> variable_t *var,
>>   {
>>       assert(obj != NULL);
>>   
>> -    size_t count = var->value_callbacks.i_entries;
>> -    if (count == 0)
>> +    callback_entry_t *entry = var->value_callbacks;
>> +    if (entry == NULL)
>>           return;
>>   
>> -    callback_entry_t *entries = var->value_callbacks.p_entries;
>>       vlc_object_internals_t *priv = vlc_internals(obj);
>>   
>>       assert(!var->b_incallback);
>>       var->b_incallback = true;
>>       vlc_mutex_unlock(&priv->var_lock);
>>   
>> -    for (size_t i = 0; i < count; i++)
>> -        entries[i].pf_value_callback(obj, name, prev, var->val,
>> -                                     entries[i].p_data);
>> +    do
>> +    {
>> +        entry->pf_value_callback(obj, name, prev, var->val, entry->p_data);
>> +        entry = entry->next;
>> +    }
>> +    while (entry != NULL);
>>   
>>       vlc_mutex_lock(&priv->var_lock);
>>       var->b_incallback = false;
>> @@ -262,20 +266,22 @@ static void TriggerListCallback(vlc_object_t *obj,
>> variable_t *var,
>>   {
>>       assert(obj != NULL);
>>   
>> -    size_t count = var->list_callbacks.i_entries;
>> -    if (count == 0)
>> +    callback_entry_t *entry = var->value_callbacks;
>> +    if (entry == NULL)
> Do you mean var->list_callbacks instead ?
>
> Did you really test this commit ? It crashes when trying to play any files.
>

I can reproduce this.

StereoModeCallback is added as vlc_value_callback and triggered as 
vlc_list_callback.

#0  0x00007ffff73e6428 in __GI_raise (sig=sig at entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff73e802a in __GI_abort () at abort.c:89
#2  0x00007ffff711457b in vlc_thread_fatal (action=0x7ffff7161430 
"locking mutex", error=35, function=0x7ffff7161658 <__func__.6484> 
"vlc_mutex_lock", file=0x7ffff7161388 "../../vlc/src/posix/thread.c", lin
e=214) at ../../vlc/src/posix/thread.c:149
#3  0x00007ffff7114a71 in vlc_mutex_lock (p_mutex=0x6a3898) at 
../../vlc/src/posix/thread.c:214
#4  0x00007ffff70c8f2e in StereoModeCallback (obj=0x6a37e0, 
varname=0x7ffff7145e91 "stereo-mode", oldval=..., newval=..., data=0x0) 
at ../../vlc/src/audio_output/output.c:181
#5  0x00007ffff71085a4 in TriggerListCallback (obj=0x6a37e0, 
var=0x688260, name=0x7ffff7145e91 "stereo-mode", action=34, val=0x0) at 
../../vlc/src/misc/variables.c:281
#6  0x00007ffff7109281 in var_Change (p_this=0x6a37e0, 
psz_name=0x7ffff7145e91 "stereo-mode", i_action=34, p_val=0x0, 
p_val2=0x0) at ../../vlc/src/misc/variables.c:534
#7  0x00007ffff70c9b18 in aout_PrepareStereoMode (aout=0x6a37e0, 
fmt=0x6a39c8, filters_cfg=0x6a3a00, 
input_chan_type=AUDIO_CHANNEL_TYPE_BITMAP, i_nb_input_channels=2) at 
../../vlc/src/audio_output/output.c:
417
#8  0x00007ffff70ca0db in aout_OutputNew (aout=0x6a37e0, fmt=0x6a39c8, 
filters_cfg=0x6a3a00) at ../../vlc/src/audio_output/output.c:583
#9  0x00007ffff70c49f1 in aout_DecNew (p_aout=0x6a37e0, 
p_format=0x7fffa414cb10, p_replay_gain=0x7fff98cd854c, 
p_request_vout=0x7fffa414cb00) at ../../vlc/src/audio_output/dec.c:88
#10 0x00007ffff7097088 in aout_update_format (p_dec=0x7fff98cd83b0) at 
../../vlc/src/input/decoder.c:372
#11 0x00007fff96624b8e in decoder_UpdateAudioFormat (dec=0x7fff98cd83b0) 
at ../../vlc/include/vlc_codec.h:380
#12 0x00007fff9662575d in DecodeBlock (p_dec=0x7fff98cd83b0, 
pp_block=0x7fffa414cc30) at ../../vlc/modules/codec/avcodec/audio.c:410
#13 0x00007fff966259c0 in DecodeAudio (p_dec=0x7fff98cd83b0, 
p_block=0x0) at ../../vlc/modules/codec/avcodec/audio.c:469
#14 0x00007ffff70998e1 in DecoderDecode (p_dec=0x7fff98cd83b0, 
p_block=0x7fff98d7cd20) at ../../vlc/src/input/decoder.c:1335
#15 0x00007ffff7099d03 in DecoderProcess (p_dec=0x7fff98cd83b0, 
p_block=0x7fff98d7cd20) at ../../vlc/src/input/decoder.c:1458
#16 0x00007ffff709a34c in DecoderThread (p_data=0x7fff98cd83b0) at 
../../vlc/src/input/decoder.c:1614
#17 0x00007ffff79866ba in start_thread (arg=0x7fffa414d700) at 
pthread_create.c:333
#18 0x00007ffff74b841d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109


>>           return;
>>   
>> -    callback_entry_t *entries = var->list_callbacks.p_entries;
>>       vlc_object_internals_t *priv = vlc_internals(obj);
>>   
>>       assert(!var->b_incallback);
>>       var->b_incallback = true;
>>       vlc_mutex_unlock(&priv->var_lock);
>>   
>> -    for (size_t i = 0; i < count; i++)
>> -        entries[i].pf_list_callback(obj, name, action, val,
>> -                                      entries[i].p_data);
>> +    do
>> +    {
>> +        entry->pf_list_callback(obj, name, action, val, entry->p_data);
>> +        entry = entry->next;
>> +    }
>> +    while (entry != NULL);
>>   
>>       vlc_mutex_lock(&priv->var_lock);
>>       var->b_incallback = false;
>> @@ -303,7 +309,7 @@ int (var_Create)( vlc_object_t *p_this, const char
>> *psz_name, int i_type )
>>       p_var->choices_text.p_values = NULL;
>>   
>>       p_var->b_incallback = false;
>> -    p_var->value_callbacks = (callback_table_t){ 0, NULL };
>> +    p_var->value_callbacks = NULL;
>>   
>>       /* Always initialize the variable, even if it is a list variable; this
>>        * will lead to errors if the variable is not initialized, but it will
>> @@ -759,7 +765,8 @@ typedef enum
>>   } vlc_callback_type_t;
>>   
>>   static void AddCallback( vlc_object_t *p_this, const char *psz_name,
>> -                        callback_entry_t entry, vlc_callback_type_t i_type )
>> +                         callback_entry_t *restrict entry,
>> +                         vlc_callback_type_t i_type )
>>   {
>>       variable_t *p_var;
>>   
>> @@ -772,18 +779,21 @@ static void AddCallback( vlc_object_t *p_this,
>> const char *psz_name,
>>       {
>>           vlc_mutex_unlock( &p_priv->var_lock );
>>           msg_Err( p_this, "cannot add callback %p to nonexistent
>> variable '%s'",
>> -                 entry.p_callback, psz_name );
>> +                 entry->p_callback, psz_name );
>>           return;
>>       }
>>   
>>       WaitUnused( p_this, p_var );
>>   
>> -    callback_table_t *p_table;
>> +    callback_entry_t **pp;
>> +
>>       if (i_type == vlc_value_callback)
>> -        p_table = &p_var->value_callbacks;
>> +        pp = &p_var->value_callbacks;
>>       else
>> -        p_table = &p_var->list_callbacks;
>> -    TAB_APPEND(p_table->i_entries, p_table->p_entries, entry);
>> +        pp = &p_var->list_callbacks;
>> +
>> +    entry->next = *pp;
>> +    *pp = entry;
>>   
>>       vlc_mutex_unlock( &p_priv->var_lock );
>>   }
>> @@ -791,21 +801,19 @@ static void AddCallback( vlc_object_t *p_this,
>> const char *psz_name,
>>   void (var_AddCallback)(vlc_object_t *p_this, const char *psz_name,
>>                          vlc_callback_t pf_callback, void *p_data)
>>   {
>> -    callback_entry_t entry;
>> -    entry.pf_value_callback = pf_callback;
>> -    entry.p_data = p_data;
>> +    callback_entry_t *entry = xmalloc(sizeof (*entry));
>>   
>> +    entry->pf_value_callback = pf_callback;
>> +    entry->p_data = p_data;
>>       AddCallback(p_this, psz_name, entry, vlc_value_callback);
>>   }
>>   
>>   static void DelCallback( vlc_object_t *p_this, const char *psz_name,
>> -                         callback_entry_t entry, vlc_callback_type_t i_type )
>> +                         const callback_entry_t *restrict match,
>> +                         vlc_callback_type_t i_type )
>>   {
>> -    int i_entry;
>> +    callback_entry_t **pp, *entry;
>>       variable_t *p_var;
>> -#ifndef NDEBUG
>> -    bool b_found_similar = false;
>> -#endif
>>   
>>       assert( p_this );
>>   
>> @@ -816,46 +824,31 @@ static void DelCallback( vlc_object_t *p_this,
>> const char *psz_name,
>>       {
>>           vlc_mutex_unlock( &p_priv->var_lock );
>>           msg_Err( p_this, "cannot delete callback %p from nonexistent "
>> -                 "variable '%s'", entry.p_callback, psz_name );
>> +                 "variable '%s'", match->p_callback, psz_name );
>>           return;
>>       }
>>   
>>       WaitUnused( p_this, p_var );
>>   
>> -    callback_table_t *p_table;
>>       if (i_type == vlc_value_callback)
>> -        p_table = &p_var->value_callbacks;
>> +        pp = &p_var->value_callbacks;
>>       else
>> -        p_table = &p_var->list_callbacks;
>> +        pp = &p_var->list_callbacks;
>>   
>> -    for( i_entry = p_table->i_entries ; i_entry-- ; )
>> -    {
>> -        if( p_table->p_entries[i_entry].p_callback == entry.p_callback
>> -            && p_table->p_entries[i_entry].p_data == entry.p_data )
>> -        {
>> -            break;
>> -        }
>> -#ifndef NDEBUG
>> -        else if( p_table->p_entries[i_entry].p_callback == entry.p_callback )
>> -            b_found_similar = true;
>> -#endif
>> -    }
>> +    entry = *pp;
>> +    assert(entry != NULL);
>>   
>> -    if( i_entry < 0 )
>> +    while (entry->p_callback != match->p_callback
>> +        || entry->p_data != match->p_data)
>>       {
>> -#ifndef NDEBUG
>> -        if( b_found_similar )
>> -            fprintf( stderr, "Calling var_DelCallback for '%s' with the same "
>> -                             "function but not the same data.", psz_name );
>> -        vlc_assert_unreachable();
>> -#endif
>> -        vlc_mutex_unlock( &p_priv->var_lock );
>> -        return;
>> +        pp = &entry->next;
>> +        entry = *pp;
>> +        assert(entry != NULL);
>>       }
>>   
>> -    TAB_ERASE(p_table->i_entries, p_table->p_entries, i_entry);
>> -
>> +    *pp = entry->next;
>>       vlc_mutex_unlock( &p_priv->var_lock );
>> +    free(entry);
>>   }
>>   
>>   void (var_DelCallback)(vlc_object_t *p_this, const char *psz_name,
>> @@ -865,7 +858,7 @@ void (var_DelCallback)(vlc_object_t *p_this, const
>> char *psz_name,
>>       entry.pf_value_callback = pf_callback;
>>       entry.p_data = p_data;
>>   
>> -    DelCallback(p_this, psz_name, entry, vlc_value_callback);
>> +    DelCallback(p_this, psz_name, &entry, vlc_value_callback);
>>   }
>>   
>>   void (var_TriggerCallback)(vlc_object_t *p_this, const char *psz_name)
>> @@ -886,10 +879,10 @@ void (var_TriggerCallback)(vlc_object_t *p_this,
>> const char *psz_name)
>>   void (var_AddListCallback)(vlc_object_t *p_this, const char *psz_name,
>>                              vlc_list_callback_t pf_callback, void
>> *p_data)
>>   {
>> -    callback_entry_t entry;
>> -    entry.pf_list_callback = pf_callback;
>> -    entry.p_data = p_data;
>> +    callback_entry_t *entry = xmalloc(sizeof (*entry));
>>   
>> +    entry->pf_list_callback = pf_callback;
>> +    entry->p_data = p_data;
>>       AddCallback(p_this, psz_name, entry, vlc_list_callback);
>>   }
>>   
>> @@ -897,10 +890,10 @@ void (var_DelListCallback)(vlc_object_t *p_this,
>> const char *psz_name,
>>                              vlc_list_callback_t pf_callback, void
>> *p_data)
>>   {
>>       callback_entry_t entry;
>> +
>>       entry.pf_list_callback = pf_callback;
>>       entry.p_data = p_data;
>> -
>> -    DelCallback(p_this, psz_name, entry, vlc_list_callback);
>> +    DelCallback(p_this, psz_name, &entry, vlc_list_callback);
>>   }
>>   
>>   /** Parse a stringified option
>> @@ -1187,8 +1180,17 @@ static void DumpVariable(const void *data, const
>> VISIT which, const int depth)
>>           fputs(", has choices", stdout);
>>       if (var->i_type & VLC_VAR_ISCOMMAND)
>>           fputs(", command", stdout);
>> -    if (var->value_callbacks.i_entries)
>> -        printf(", %d callbacks", var->value_callbacks.i_entries);
>> +    if (var->value_callbacks != NULL)
>> +    {
>> +        size_t count = 0;
>> +
>> +        for (callback_entry_t *entry = var->value_callbacks;
>> +             entry != NULL;
>> +             entry = entry->next)
>> +            count++;
>> +
>> +        printf(", %zu callbacks", count);
>> +    }
>>   
>>       switch (var->i_type & VLC_VAR_CLASS)
>>       {
>>
>> _______________________________________________
>> vlc-commits mailing list
>> vlc-commits at videolan.org
>> https://mailman.videolan.org/listinfo/vlc-commits
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



More information about the vlc-devel mailing list