[vlc-devel] [PATCH 2/2] decoder: always release the vout

[Alexandre Janniaux]

Hi,

If I understand correctly, input_resource_GetVoutDecoderDevice
is setting p_resource->p_vout_free to NULL, and then we create
a vout from there following the ModuleThread_UpdateVideoFormat
and call input_resource_StartVout from there, which might fail.

The return value of input_resource_StartVout is also the return
value of ModuleThread_UpdateVideoFormat, meaning that the
decoder could close while the assertion is not valid anymore
because we didn't store the p_vout_free value since it was not
the primary vout (pp_vout[0] != vout).

Your patch would fix this case, but I'm not sure which path
leads to having the main vout not being stored as the free
vout if it's available. Is it when starting a second decoder
after the first failed to update format and closed?

You could add a comment above the assertion explaining the
reason too! Thanks for fixing this issue.

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, Jun 26, 2020 at 01:26:02PM +0200, Thomas Guillem wrote:
> The vout was released only when already started. This fixes the
> "assert( p_resource->i_vout == 0 || p_resource->p_vout_free == p_resource->pp_vout[0] );"
> from input_resource_SetInput() when the vout failed to start.
> ---
>  src/input/decoder.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index dd901f77b0c..57b13c14fb3 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -2001,15 +2001,18 @@ static void DeleteDecoder( vlc_input_decoder_t *p_owner )
>          case VIDEO_ES: {
>              vout_thread_t *vout = p_owner->p_vout;
>
> -            if (vout != NULL && p_owner->vout_thread_started)
> +            if (vout != NULL)
>              {
> -                /* Reset the cancel state that was set before joining the decoder
> -                 * thread */
> -                if (p_owner->out_pool)
> -                    picture_pool_Cancel( p_owner->out_pool, false );
> -                vout_StopDisplay(vout);
> -                p_owner->vout_thread_started = false;
> -                decoder_Notify(p_owner, on_vout_stopped, vout);
> +                if( p_owner->vout_thread_started)
> +                {
> +                    /* Reset the cancel state that was set before joining the
> +                     * decoder thread */
> +                    if (p_owner->out_pool)
> +                        picture_pool_Cancel( p_owner->out_pool, false );
> +                    vout_StopDisplay(vout);
> +                    p_owner->vout_thread_started = false;
> +                    decoder_Notify(p_owner, on_vout_stopped, vout);
> +                }
>                  input_resource_PutVout(p_owner->p_resource, vout);
>              }
>              break;
> --
> 2.20.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel