[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