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

[Steve Lhomme]

On 2020-02-07 14:37, Alexandre Janniaux wrote:
> 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?

Yes.

>>
>> 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.

Ah indeed. I thought a filter would be added if the formats don't match 
(what you're trying to do). I think it used to.

>>
>> 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.

I don't dully understand this chain of 6 filter chain nor what this 
downstream_id is. But it does seem to be missing what you're trying to add.

> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>