[vlc-devel] [PATCH 2/7] transcode: video: recreate the filters once we know the encoder input format

[Alexandre Janniaux]

Hi,

Sorry for the late answer.

This patch seems almost ok in the short time, if you want to
merge with the few corrections.

However I don't completely agree to the direction it's leading.
If you agree with the following, an addition to the commit
message mentionning it will be useful for the future.

Decoder device is supposed to be hinting the best pipeline for
the output already, so it seems not correct to try restarting
until it's not optimal.

There is an issue with the current design in transcode, which
is part of what I've been working on. We can start with the
following commit as a context:

0d05d0d528ce785096d62abad59324bc69534c28
transcode: video: remove write-only decoder_vctx_out variable

When opening the transcode pipeline for a stream_id, the
encoder is tested and created along with the filters, but
the video context is not available at this location.

Instead, both should be done from the first decoder_updateVideoOuput
or format, which is in the playback pipeline what is creating the
video output.

transcode_video_process should probably never update the
filters.

Then, p_final_conv_static was also added because filter format
can be incompatible, and in your case even after recreating the
filters you could have a different format between the filters
and the encoders, since user filters are not forced to match
the output format and a converter is then expected.

In particular, filters_init() ends by using
transcode_encoder_update_format_in which change the encoder format
without notifying the encoders, leading to the reintroduction of
#22703 crash.

So this patch should take this into account before merging.

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, Aug 14, 2020 at 04:00:24PM +0200, Steve Lhomme wrote:
> We may get better filters if the encoder can handle GPU (which is not known
> when test creating the encoder with no decoder).
>
> This makes the p_final_conv_static filter useless as we always have the proper
> output of the filter chain.
> ---
>  modules/stream_out/transcode/video.c | 37 +++++++++++-----------------
>  1 file changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/modules/stream_out/transcode/video.c b/modules/stream_out/transcode/video.c
> index f5100184c35..0e5f589b417 100644
> --- a/modules/stream_out/transcode/video.c
> +++ b/modules/stream_out/transcode/video.c
> @@ -572,6 +572,7 @@ int transcode_video_process( sout_stream_t *p_stream, sout_stream_id_sys_t *id,
>              transcode_video_framerate_apply( &p_pic->format, &id->decoder_out.video );
>              transcode_video_sar_apply( &p_pic->format, &id->decoder_out.video );
>
> +            bool need_filters = false;
>              if( !transcode_video_filters_configured( id ) )
>              {
>                  if( transcode_video_filters_init( p_stream,
> @@ -582,13 +583,9 @@ int transcode_video_process( sout_stream_t *p_stream, sout_stream_id_sys_t *id,
>                                                   transcode_encoder_format_in( id->encoder ),
>                                                   id ) != VLC_SUCCESS )
>                      goto error;
> +                need_filters = true;
>              }
>
> -            /* Store the current encoder input chroma to detect whether we need
> -             * a converter in p_final_conv_static. The encoder will override it
> -             * if it needs any different format or chroma. */
> -            es_format_t filter_fmt_out;
> -            es_format_Copy( &filter_fmt_out, transcode_encoder_format_in( id->encoder ) );
>              bool is_encoder_open = transcode_encoder_opened( id->encoder );
>
>              /* Start missing encoder */
> @@ -602,25 +599,21 @@ int transcode_video_process( sout_stream_t *p_stream, sout_stream_id_sys_t *id,
>                  goto error;
>              }
>
> -            /* The fmt_in may have been overriden by the encoder. */
> -            const es_format_t *encoder_fmt_in = transcode_encoder_format_in( id->encoder );
> -
> -            /* In case the encoder wasn't open yet, check if we need to add
> -             * a converter between last user filter and encoder. */
> -            if( !is_encoder_open &&
> -                filter_fmt_out.i_codec != encoder_fmt_in->i_codec )
> +            if ( need_filters || !is_encoder_open )
>              {
> -                if ( !id->p_final_conv_static )
> -                    id->p_final_conv_static =
> -                        filter_chain_NewVideo( p_stream, false, NULL );
> -                filter_chain_Reset( id->p_final_conv_static,
> -                                    &filter_fmt_out,
> -                                    //encoder_vctx_in,
> -                                    NULL,
> -                                    encoder_fmt_in );
> -                filter_chain_AppendConverter( id->p_final_conv_static, NULL );
> +                transcode_video_filters_clean( id );
> +                if( transcode_video_filters_init( p_stream,
> +                                                  id->p_filterscfg,
> +                                                 (id->p_enccfg->video.fps.num > 0),
> +                                                 &id->decoder_out,
> +                                                 picture_GetVideoContext(p_pic),
> +                                                 transcode_encoder_format_in( id->encoder ),
> +                                                 id ) != VLC_SUCCESS )
> +                {
> +                    msg_Err( p_stream, "failed to recreate filters" );
> +                    goto error;
> +                }
>              }
> -            es_format_Clean(&filter_fmt_out);
>
>              msg_Dbg( p_stream, "destination (after video filters) %ux%u",
>                                 transcode_encoder_format_in( id->encoder )->video.i_width,
> --
> 2.26.2
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel