[vlc-devel] [PATCH] decoder: avoid vout requests spam

[Rémi Denis-Courmont]

Le 1 juillet 2017 17:44:30 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>
>
>On Sat, Jul 1, 2017, at 17:33, Rémi Denis-Courmont wrote:
>> Le 1 juillet 2017 16:54:42 GMT+02:00, Thomas Guillem <thomas at gllm.fr>
>> a écrit :>> ---
>>> 
>>>  src/input/decoder.c | 20 ++++++++++++++++++++
>>> 
>>>  1 file changed, 20 insertions(+)
>>> 
>>> 
>>> 
>>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>>> 
>>> index dac313737a..23e5a7356b 100644
>>> 
>>> --- a/src/input/decoder.c
>>> 
>>> +++ b/src/input/decoder.c
>>> 
>>> @@ -54,6 +54,11 @@
>>> 
>>>  
>>> 
>>>  #include "../video_output/vout_control.h"
>>> 
>>>  
>>> 
>>> +/* Number of vout request failures after we slow down vout
>>> requests */
>>>>> +#define VOUT_MAX_FAILURE 10
>>> 
>>> +/* Delay between each failing vout requests */
>>> 
>>> +#define VOUT_RETRY_DELAY 1000000UL
>>> 
>>> +
>>> 
>>>  /*
>>> 
>>>   * Possibles values set in p_owner->reload atomic
>>> 
>>>   */
>>> 
>>> @@ -108,6 +113,8 @@ struct decoder_owner_sys_t
>>> 
>>>      audio_output_t *p_aout;
>>> 
>>>  
>>> 
>>>      vout_thread_t   *p_vout;
>>> 
>>> +    unsigned         vout_nb_failures;
>>> 
>>> +    mtime_t          vout_last_failure;
>>> 
>>>  
>>> 
>>>      /* -- Theses variables need locking on read *and* write -- */
>>>>>      /* Preroll */
>>> 
>>> @@ -408,6 +415,13 @@ static int vout_update_format( decoder_t
>>> *p_dec )
>>>>>  {
>>> 
>>>      decoder_owner_sys_t *p_owner = p_dec->p_owner;
>>> 
>>>  
>>> 
>>> +    /* Avoid to spam vout requests if no vout is capable of
>handling
>>>      the
>>>>> +     * current format. We should still request vouts from time to
>>>       time since a
>>>>> +     * vout can become available in between. */
>>> 
>>> +    if( p_owner->vout_nb_failures > VOUT_MAX_FAILURE
>>> 
>>> +     && mdate() - p_owner->vout_last_failure < VOUT_RETRY_DELAY )
>>>>> +        return -1;
>>> 
>>> +
>>> 
>>>      if( p_owner->p_vout == NULL
>>> 
>>>       || p_dec->fmt_out.video.i_width != p_owner->fmt.video.i_width
>>>>>       || p_dec->fmt_out.video.i_height !=
>p_owner->fmt.video.i_height
>>>>> @@ -530,8 +544,12 @@ static int vout_update_format( decoder_t
>>> *p_dec )
>>>>>          if( p_vout == NULL )
>>> 
>>>          {
>>> 
>>>              msg_Err( p_dec, "failed to create video output" );
>>> 
>>> +            p_owner->vout_nb_failures++;
>>> 
>>> +            p_owner->vout_last_failure = mdate();
>>> 
>>>              return -1;
>>> 
>>>          }
>>> 
>>> +        p_owner->vout_nb_failures = 0;
>>> 
>>> +        p_owner->vout_last_failure = 0;
>>> 
>>>      }
>>> 
>>>  
>>> 
>>>      if ( memcmp( &p_dec->fmt_out.video.mastering,
>>> 
>>> @@ -1662,6 +1680,8 @@ static decoder_t * CreateDecoder( vlc_object_t
>>> *p_parent,
>>>>>      p_owner->p_resource = p_resource;
>>> 
>>>      p_owner->p_aout = NULL;
>>> 
>>>      p_owner->p_vout = NULL;
>>> 
>>> +    p_owner->vout_nb_failures = 0;
>>> 
>>> +    p_owner->vout_last_failure = 0;
>>> 
>>>      p_owner->p_spu_vout = NULL;
>>> 
>>>      p_owner->i_spu_channel = 0;
>>> 
>>>      p_owner->i_spu_order = 0;
>>> 
>> 
>> AFAICT, retrying does not make sense. Just fail immediately if the
>> format is the same as before. It cannot be assumed that the decoder
>> will actually keep retrying with the same format (even now, that is
>> not always true).
>Fail immediately ? I was in favor of that solution but someone
>(maybe you ?) told me that we should always retry because we can't
>know the reason of the failure. This could be because of low memory,
>a driver/display not available that can became available just after
>few seconds.
>Even with that in mind, I prefer to fail immediately than doing this
>delay hack.
>> 
>>  With that said, you can't expect this to really prevent spam. The
>>  decoder could just as well iterate multiple failing formats again
>and
>>  again. So I'd say this patch is kinda useless.>  -- 
>>  Rémi Denis-Courmont
>>  Typed on an inconvenient virtual keyboard 
>> _________________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel

We cannot really assume anything at that level either way. This would be second-guessing the decoder and the video outputs.

You could rate limit the error message though.
-- 
Rémi Denis-Courmont
Typed on an inconvenient virtual keyboard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170701/4b882550/attachment.html>