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

[Thomas Guillem]

On Sat, Jul 1, 2017, at 17:46, Rémi Denis-Courmont wrote:
> 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.

The main problem is the window creation/deletion glitch, not the
error message.
You can easily reproduce it with ./vlc --vout sdl for example. (keep in
mind that this problem can happen  without forcing any vout).
>  -- 
>  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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170701/6ef6effd/attachment.html>