[vlc-devel] [PATCH 02/11] demux: add helper functions to avoid accessing update data directly outside of demux.c

Steve Lhomme robux4 at gmail.com
Mon Jun 6 19:52:00 CEST 2016


On Mon, Jun 6, 2016 at 6:43 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le 2016-06-06 16:49, Steve Lhomme a écrit :
>>
>> only demux.c code and the demuxer itself should access these data directly
>> ---
>>  src/input/demux.c        | 19 +++++++++++++++++++
>>  src/input/demux.h        |  6 ++++++
>>  src/input/input.c        | 32 ++++++++++++++++----------------
>>  src/input/stream_demux.c |  4 ++--
>>  4 files changed, 43 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/input/demux.c b/src/input/demux.c
>> index c532752..baf1880 100644
>> --- a/src/input/demux.c
>> +++ b/src/input/demux.c
>> @@ -656,3 +656,22 @@ static bool SkipAPETag( demux_t *p_demux )
>>      return true;
>>  }
>>
>> +int demux_GetUpdateFlags( demux_t *p_demux )
>> +{
>> +    return p_demux->info.i_update;
>> +}
>> +
>> +void demux_ResetUpdateFlags( demux_t *p_demux, int i_flags )
>> +{
>> +    p_demux->info.i_update &= ~i_flags;
>> +}
>
>
> Is there an actual point in having two layers of function calls?

You mean Get+Reset ? I originally planned to go with ConsumeFlags()
but in stream_demux.c i_update is used with a lock on modified data in
between the get and the reset. Not knowing the side effects, I
preferred to keep it that way for now.

> I would
> remove the flags completely, query the title and seekpoint, and then compare
> with previous values.

That means storing the previous value somewhere.

i_update is also use with INPUT_UPDATE_META and
INPUT_UPDATE_TITLE_LIST which don't correspond to a particular value
to compare.

>> +
>> +int demux_GetTitle( demux_t *p_demux )
>> +{
>> +    return p_demux->info.i_title;
>> +}
>> +
>> +int demux_GetSeekpoint( demux_t *p_demux )
>> +{
>> +    return p_demux->info.i_seekpoint;
>> +}
>> diff --git a/src/input/demux.h b/src/input/demux.h
>> index f841f68..2e5b01a 100644
>> --- a/src/input/demux.h
>> +++ b/src/input/demux.h
>> @@ -39,4 +39,10 @@ demux_t *demux_NewAdvanced( vlc_object_t *p_obj,
>> input_thread_t *p_parent_input,
>>  demux_t *input_DemuxNew( vlc_object_t *, const char *access, const
>> char *demux,
>>                           const char *path, es_out_t *out, bool quick,
>>                           input_thread_t * );
>> +
>> +int demux_GetUpdateFlags( demux_t * );
>> +void demux_ResetUpdateFlags( demux_t *, int );
>> +int demux_GetTitle( demux_t * );
>> +int demux_GetSeekpoint( demux_t * );
>> +
>>  #endif
>> diff --git a/src/input/input.c b/src/input/input.c
>> index a5eeb57..92a433c 100644
>> --- a/src/input/input.c
>> +++ b/src/input/input.c
>> @@ -562,12 +562,12 @@ static void MainLoopDemux( input_thread_t
>> *p_input, bool *pb_changed )
>>
>>      if( i_ret == VLC_DEMUXER_SUCCESS )
>>      {
>> -        if( p_demux->info.i_update )
>> +        if( demux_GetUpdateFlags( p_demux ) )
>>          {
>> -            if( p_demux->info.i_update & INPUT_UPDATE_TITLE_LIST )
>> +            if( demux_GetUpdateFlags( p_demux ) & INPUT_UPDATE_TITLE_LIST
>> )
>>              {
>>                  UpdateTitleListfromDemux( p_input );
>> -                p_demux->info.i_update &= ~INPUT_UPDATE_TITLE_LIST;
>> +                demux_ResetUpdateFlags( p_demux, INPUT_UPDATE_TITLE_LIST
>> );
>>              }
>>              if( p_input->p->master->b_title_demux )
>>              {
>> @@ -1927,7 +1927,7 @@ static bool Control( input_thread_t *p_input,
>>              if( p_input->p->master->i_title <= 0 )
>>                  break;
>>
>> -            int i_title = p_input->p->master->p_demux->info.i_title;
>> +            int i_title = demux_GetTitle( p_input->p->master->p_demux );
>>              if( i_type == INPUT_CONTROL_SET_TITLE_PREV )
>>                  i_title--;
>>              else if( i_type == INPUT_CONTROL_SET_TITLE_NEXT )
>> @@ -1957,8 +1957,8 @@ static bool Control( input_thread_t *p_input,
>>
>>              demux_t *p_demux = p_input->p->master->p_demux;
>>
>> -            int i_title = p_demux->info.i_title;
>> -            int i_seekpoint = p_demux->info.i_seekpoint;
>> +            int i_title = demux_GetTitle( p_demux );
>> +            int i_seekpoint = demux_GetSeekpoint( p_demux );
>>
>>              if( i_type == INPUT_CONTROL_SET_SEEKPOINT_PREV )
>>              {
>> @@ -2175,33 +2175,33 @@ static int UpdateTitleSeekpointFromDemux(
>> input_thread_t *p_input )
>>      demux_t *p_demux = p_input->p->master->p_demux;
>>
>>      /* TODO event-like */
>> -    if( p_demux->info.i_update & INPUT_UPDATE_TITLE )
>> +    if( demux_GetUpdateFlags( p_demux ) & INPUT_UPDATE_TITLE )
>>      {
>> -        input_SendEventTitle( p_input, p_demux->info.i_title );
>> +        input_SendEventTitle( p_input, demux_GetTitle( p_demux ) );
>>
>> -        p_demux->info.i_update &= ~INPUT_UPDATE_TITLE;
>> +        demux_ResetUpdateFlags( p_demux, INPUT_UPDATE_TITLE );
>>      }
>> -    if( p_demux->info.i_update & INPUT_UPDATE_SEEKPOINT )
>> +    if( demux_GetUpdateFlags( p_demux ) & INPUT_UPDATE_SEEKPOINT )
>>      {
>>          input_SendEventSeekpoint( p_input,
>> -                                  p_demux->info.i_title,
>> p_demux->info.i_seekpoint );
>> +                                  demux_GetTitle( p_demux ),
>> demux_GetSeekpoint( p_demux ) );
>>
>> -        p_demux->info.i_update &= ~INPUT_UPDATE_SEEKPOINT;
>> +        demux_ResetUpdateFlags( p_demux, INPUT_UPDATE_SEEKPOINT );
>>      }
>>
>>      return UpdateTitleSeekpoint( p_input,
>> -                                 p_demux->info.i_title,
>> -                                 p_demux->info.i_seekpoint );
>> +                                 demux_GetTitle( p_demux ),
>> +                                 demux_GetSeekpoint( p_demux ) );
>>  }
>>
>>  static void UpdateGenericFromDemux( input_thread_t *p_input )
>>  {
>>      demux_t *p_demux = p_input->p->master->p_demux;
>>
>> -    if( p_demux->info.i_update & INPUT_UPDATE_META )
>> +    if( demux_GetUpdateFlags( p_demux ) & INPUT_UPDATE_META )
>>      {
>>          InputUpdateMeta( p_input, p_demux );
>> -        p_demux->info.i_update &= ~INPUT_UPDATE_META;
>> +        demux_ResetUpdateFlags( p_demux, INPUT_UPDATE_META );
>>      }
>>      {
>>          double quality;
>> diff --git a/src/input/stream_demux.c b/src/input/stream_demux.c
>> index 76ca3e3..f87c7f6 100644
>> --- a/src/input/stream_demux.c
>> +++ b/src/input/stream_demux.c
>> @@ -261,7 +261,7 @@ static void* DStreamThread( void *obj )
>>      mtime_t next_update = 0;
>>      while( atomic_load( &p_sys->active ) )
>>      {
>> -        if( p_demux->info.i_update || mdate() >= next_update )
>> +        if( demux_GetUpdateFlags( p_demux ) || mdate() >= next_update )
>>          {
>>              double newpos;
>>              int64_t newlen, newtime;
>> @@ -279,7 +279,7 @@ static void* DStreamThread( void *obj )
>>              p_sys->stats.time = newtime;
>>              vlc_mutex_unlock( &p_sys->lock );
>>
>> -            p_demux->info.i_update = 0;
>> +            demux_ResetUpdateFlags( p_demux, 0xFFFF );
>>              next_update = mdate() + (CLOCK_FREQ / 4);
>>          }
>
>
> --
> 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


More information about the vlc-devel mailing list