[vlc-devel] [PATCH 1/8] decoder: simplify how we report lost pictures

[Steve Lhomme]

On 2019-08-27 9:57, Thomas Guillem wrote:
> 
> 
> On Tue, Aug 27, 2019, at 07:28, Steve Lhomme wrote:
>> No need for pointers.
>> ---
>>   src/input/decoder.c | 16 +++++++---------
>>   1 file changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> index 782dc0a422..d05351fa5c 100644
>> --- a/src/input/decoder.c
>> +++ b/src/input/decoder.c
>> @@ -981,10 +981,9 @@ static void DecoderQueueCc( decoder_t *p_videodec,
>> block_t *p_cc,
>>       }
>>   }
>>   
>> -static void DecoderPlayVideo( decoder_t *p_dec, picture_t *p_picture,
>> -                             unsigned *restrict pi_lost_sum )
> 
> Why ? Every functions in this file take a decoder_t* in argument, you should not change it wihtout explaining why.

I actually have a further patch that passes the owner directly if it's 
an internal call. I guess I should do the separation better.

>> +static bool DecoderPlayVideo( struct decoder_owner *p_owner, picture_t
>> *p_picture )
> 
> You should return a int error instead. Every functions of this file do that, it feel weird that only one function is returning a bool for error checking.

OK

> 
>>   {
>> -    struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> +    decoder_t *p_dec = &p_owner->dec;
>>       vout_thread_t  *p_vout = p_owner->p_vout;
>>       bool prerolled;
>>   
>> @@ -993,7 +992,7 @@ static void DecoderPlayVideo( decoder_t *p_dec,
>> picture_t *p_picture,
>>       {
>>           vlc_mutex_unlock( &p_owner->lock );
>>           picture_Release( p_picture );
>> -        return;
>> +        return false;
>>       }
>>   
>>       prerolled = p_owner->i_preroll_end > (vlc_tick_t)INT64_MIN;
>> @@ -1067,10 +1066,10 @@ static void DecoderPlayVideo( decoder_t *p_dec,
>> picture_t *p_picture,
>>           goto discard;
>>       }
>>   
>> -    return;
>> +    return false;
>>   discard:
>> -    *pi_lost_sum += 1;
>>       picture_Release( p_picture );
>> +    return true;
>>   }
>>   
>>   static void DecoderUpdateStatVideo( struct decoder_owner *p_owner,
>> @@ -1092,12 +1091,11 @@ static void DecoderUpdateStatVideo( struct
>> decoder_owner *p_owner,
>>   static void DecoderQueueVideo( decoder_t *p_dec, picture_t *p_pic )
>>   {
>>       assert( p_pic );
>> -    unsigned i_lost = 0;
>>       struct decoder_owner *p_owner = dec_get_owner( p_dec );
>>   
>> -    DecoderPlayVideo( p_dec, p_pic, &i_lost );
>> +    bool lost = DecoderPlayVideo( p_owner, p_pic );
>>   
>> -    p_owner->pf_update_stat( p_owner, 1, i_lost );
>> +    p_owner->pf_update_stat( p_owner, 1, lost ? 1 : 0 );
>>   }
>>   
>>   static int thumbnailer_update_format( decoder_t *p_dec )
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>