[vlc-devel] [PATCH 1/2] transcode: video: append converter before encoder if needed

[Alexandre Janniaux]

Hi,

On Fri, Feb 07, 2020 at 01:37:24PM +0100, Steve Lhomme wrote:
> Hi,
>
> It's that what this piece of code does right after the encoder is opened ?

You mean whether the code you quoted should already be
handling what my patch is doing or not?

>
> msg_Dbg( p_stream, "destination (after video filters) %ux%u",
>                     transcode_encoder_format_in( id->encoder
> )->video.i_width,
>                     transcode_encoder_format_in( id->encoder
> )->video.i_height );

This part is only logs.

>
> if( !id->downstream_id )
>     id->downstream_id =
>         id->pf_transcode_downstream_add( p_stream,
>                                             &id->p_decoder->fmt_in,
>
> transcode_encoder_format_out( id->encoder ) );

And this part is to link transcode/video.c code handling
video filtering/transcoding stuff-related with the
trancode/transcode.c which handle the ES streams themselves.

The transcode/video.c use the transcode/encoder/ code to
configure the encoder, and I'm adding an additional
configuration step between trancode/video.c and
trancode/encoder. Thus, it cannot be done in the
pf_transcode_downstream_add.

I tested the code with a filter using GBM to run the OpenGL
pipeline, so my filter returned VLC_CODEC_GBM frames:
 - without this patch, the VLC_CODEC_GBM are not converted
but still manipulated like if they were VLC_CODEC_I420 frames.
 - with my patch, the VLC_CODEC_GBM -> VLC_CODEC_RGB32 filter
is correctly appended, followed by another conversion to
VLC_CODEC_I420.

I hope it matches what you asked,

Regards,
--
Alexandre Janniaux
Videolabs

>
>
> On 2020-02-07 13:17, Alexandre Janniaux wrote:
> > The encoder is able to override its input format, especially to change
> > the input chroma. The semantic is that the encoder user should either
> > change the input format or refuse the module probing.
> >
> > The transcode video pipeline had a dedicated filter_chain to handle such
> > conversion but it was never initialized.
> >
> > Store encoder input format before it is initialized, and initialize the
> > final filter chain if the format is different from what the encoder
> > requested.
> >
> > Fix #22703
> > ---
> >   modules/stream_out/transcode/video.c | 29 +++++++++++++++++++++++++++-
> >   1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/modules/stream_out/transcode/video.c b/modules/stream_out/transcode/video.c
> > index 8453fb70f0..592aaf54e9 100644
> > --- a/modules/stream_out/transcode/video.c
> > +++ b/modules/stream_out/transcode/video.c
> > @@ -623,8 +623,15 @@ int transcode_video_process( sout_stream_t *p_stream, sout_stream_id_sys_t *id,
> >                       goto error;
> >               }
> > +            /* 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 */
> > -            if( !transcode_encoder_opened( id->encoder ) &&
> > +            if( !is_encoder_open &&
> >                   transcode_encoder_open( id->encoder, id->p_enccfg ) != VLC_SUCCESS )
> >               {
> >                   msg_Err( p_stream, "cannot find video encoder (module:%s fourcc:%4.4s). "
> > @@ -634,6 +641,26 @@ 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 ( !id->p_final_conv_static )
> > +                    id->p_final_conv_static =
> > +                        filter_chain_NewVideo( p_stream, false, NULL );
>
> You're not checking the NULL result.
>
> > +                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 );
> > +            }
> > +            es_format_Clean(&filter_fmt_out);
>
> It should probably be cleaned in the "error" label.
>
> > +
> >               msg_Dbg( p_stream, "destination (after video filters) %ux%u",
> >                                  transcode_encoder_format_in( id->encoder )->video.i_width,
> >                                  transcode_encoder_format_in( id->encoder )->video.i_height );
> > --
> > 2.25.0
> >
> > _______________________________________________
> > 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