[vlc-devel] [PATCH 1/8] transcode: split loop transcode_video_set_conversions()

[Steve Lhomme]

On 2020-10-05 15:20, Alexandre Janniaux wrote:
> Hi,
> 
> As mentioned in private, I have really mixed feeling about this patchset since
> it is basically a revert of 4c5240a5bf3cc4c2e0f03b9c988f274c0a4d7404 in eight
> patches.

I thought the issue was that you already had modified the transcode 
heavily and didn't want troubles with a rebase. I did the rebase for you.

It's not a revert of 4c5240a5bf3cc4c2e0f03b9c988f274c0a4d7404 as the 
original issue was that the orientation wasn't applied when transcoding. 
The new code still has the fix, but in the same way the display module 
does it.

And it's in eight patches so it's easier to follow the process of why 
each part can be removed (mostly dead code). I even added more comments 
in the pushed version. I actually didn't expect to get dead code by 
moving this plus it's a pleasant surprise.

> I'd also like https://patches.videolan.org/patch/28322/ to be merged instead
> of fixing this silently.

Well, it's done (and fixed without even having to wonder about side 
effects).

> I've been working on testing the transcode pipeline for a specific case with
> video context and GPU decoders/encoders, I can try add such behaviour but
> before dropping or reverting older patchset, it seems clear to me that we
> actually need to specify what we actually want here.

Not sure what there is to discuss. We want working code and consistent 
code throughout the code base.

> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Thu, Sep 24, 2020 at 11:50:20AM +0200, Steve Lhomme wrote:
>> Rather than a loop doing things differently in both passes.
>> ---
>>   modules/stream_out/transcode/transcode.h |  4 +--
>>   modules/stream_out/transcode/video.c     | 44 +++++++++++++++++++++++-
>>   2 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/modules/stream_out/transcode/transcode.h b/modules/stream_out/transcode/transcode.h
>> index 315c5173a0d..e462d3727d5 100644
>> --- a/modules/stream_out/transcode/transcode.h
>> +++ b/modules/stream_out/transcode/transcode.h
>> @@ -121,11 +121,11 @@ struct sout_stream_id_sys_t
>>       {
>>            struct
>>            {
>> -             filter_chain_t  *p_f_chain; /**< Video filters */
>> +             filter_chain_t  *p_f_chain; /**< deinterlace & fps video filters */
>>                filter_chain_t  *p_conv_nonstatic;
>>                filter_chain_t  *p_conv_static;
>>                filter_chain_t  *p_uf_chain; /**< User-specified video filters */
>> -             filter_chain_t  *p_final_conv_static;
>> +             filter_chain_t  *p_final_conv_static; /**< converter to adapt filtered pics to the encoder */
>>                vlc_blender_t   *p_spu_blender;
>>                spu_t           *p_spu;
>>                vlc_decoder_device *dec_dev;
>> diff --git a/modules/stream_out/transcode/video.c b/modules/stream_out/transcode/video.c
>> index dc07278e11b..a2e0efa3359 100644
>> --- a/modules/stream_out/transcode/video.c
>> +++ b/modules/stream_out/transcode/video.c
>> @@ -284,7 +284,49 @@ static int transcode_video_set_conversions( sout_stream_t *p_stream,
>>           STEP_NONSTATIC = 0,
>>           STEP_STATIC,
>>       };
>> -    for( int step = STEP_NONSTATIC; step <= STEP_STATIC; step++ )
>> +    int step = STEP_NONSTATIC;
>> +    {
>> +        const bool b_do_scale = (*pp_src)->video.i_width != p_dst->video.i_width ||
>> +                                (*pp_src)->video.i_height != p_dst->video.i_height;
>> +        const bool b_do_chroma = (*pp_src)->video.i_chroma != p_dst->video.i_chroma;
>> +        const bool b_do_orient = ((*pp_src)->video.orientation != ORIENT_NORMAL) && b_reorient;
>> +
>> +        if( step == STEP_STATIC && b_do_orient )
>> +            return VLC_EGENERIC;
>> +
>> +        const es_format_t *p_tmpdst = p_dst;
>> +
>> +        if( ! (b_do_scale || b_do_chroma || b_do_orient) )
>> +            return VLC_SUCCESS;
>> +
>> +        es_format_t tmpdst;
>> +        if( b_do_orient )
>> +        {
>> +            es_format_Init( &tmpdst, VIDEO_ES, p_dst->video.i_chroma );
>> +            video_format_ApplyRotation( &tmpdst.video, &p_dst->video );
>> +            p_tmpdst = &tmpdst;
>> +        }
>> +
>> +        msg_Dbg( p_stream, "adding (scale %d,chroma %d, orient %d) converters",
>> +                 b_do_scale, b_do_chroma, b_do_orient );
>> +
>> +        filter_chain_t **pp_chain = (step == STEP_NONSTATIC)
>> +                ? &id->p_conv_nonstatic
>> +                : &id->p_conv_static;
>> +
>> +        *pp_chain = filter_chain_NewVideo( p_stream, step == STEP_NONSTATIC, &owner );
>> +        if( !*pp_chain )
>> +            return VLC_EGENERIC;
>> +        filter_chain_Reset( *pp_chain, *pp_src, *pp_src_vctx, p_tmpdst );
>> +
>> +        if( filter_chain_AppendConverter( *pp_chain, p_tmpdst ) != VLC_SUCCESS )
>> +            return VLC_EGENERIC;
>> +
>> +        *pp_src = filter_chain_GetFmtOut( *pp_chain );
>> +        *pp_src_vctx = filter_chain_GetVideoCtxOut( *pp_chain );
>> +        debug_format( p_stream, *pp_src );
>> +    }
>> +    step = STEP_STATIC;
>>       {
>>           const bool b_do_scale = (*pp_src)->video.i_width != p_dst->video.i_width ||
>>                                   (*pp_src)->video.i_height != p_dst->video.i_height;
>> --
>> 2.26.2
>>
>> _______________________________________________
>> 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
>