[vlc-devel] [PATCH] chroma_chain: try transform on smallest surface

[Alexandre Janniaux]

Hi,

The `i_first_step` looks weird in the code, maybe readability could be
improved by replacing the `i_first_step` variable and the for loop by a
xor-like operation and a separate function ?

I'm not sure to understand what this code does or fixes though, so maybe
a better commit message can be written to describe why this is needed.

Regards,
--
Alexandre Janniaux
Videolabs

On Tue, Sep 03, 2019 at 02:38:53PM +0200, Francois Cartegnie wrote:
> ---
>  modules/video_chroma/chain.c | 50 +++++++++++++++++++++---------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/modules/video_chroma/chain.c b/modules/video_chroma/chain.c
> index 8b183d2686..4eff623e96 100644
> --- a/modules/video_chroma/chain.c
> +++ b/modules/video_chroma/chain.c
> @@ -281,30 +281,38 @@ static picture_t *Chain( filter_t *p_filter, picture_t *p_pic )
>
>  static int BuildTransformChain( filter_t *p_filter )
>  {
> +    const bool b_upscaling = (uint64_t) p_filter->fmt_in.video.i_width *
> +                                        p_filter->fmt_in.video.i_height <=
> +                             (uint64_t) p_filter->fmt_out.video.i_width *
> +                                        p_filter->fmt_out.video.i_height;
> +    const int i_first_step = b_upscaling ? 0 : 1;
>
> -    es_format_t fmt_mid;
> -    int i_ret;
> -
> -    /* Lets try transform first, then (potentially) resize+chroma */
> -    msg_Dbg( p_filter, "Trying to build transform, then chroma+resize" );
> -    es_format_Copy( &fmt_mid, &p_filter->fmt_in );
> -    video_format_TransformTo(&fmt_mid.video, p_filter->fmt_out.video.orientation);
> -    i_ret = CreateChain( p_filter, &fmt_mid );
> -    es_format_Clean( &fmt_mid );
> -    if( i_ret == VLC_SUCCESS )
> -        return VLC_SUCCESS;
> +    for( int i=0; i<2; i++ )
> +    {
> +        es_format_t fmt_mid;
>
> -    /* Lets try resize+chroma first, then transform */
> -    msg_Dbg( p_filter, "Trying to build chroma+resize, then transform" );
> -    es_format_Copy( &fmt_mid, &p_filter->fmt_out );
> -    /* Set mid format to same rotation as source,
> -     * so we get proper scaling before transform */
> -    video_format_TransformTo( &fmt_mid.video, p_filter->fmt_in.video.orientation );
> -    i_ret = CreateChain( p_filter, &fmt_mid );
> -    es_format_Clean( &fmt_mid );
> -    if( i_ret == VLC_SUCCESS )
> -        return VLC_SUCCESS;
> +        if( (i_first_step + i) % 2 == 0 )
> +        {
> +            /* Lets try transform first, then (potentially) resize+chroma */
> +            msg_Dbg( p_filter, "Trying to build transform, then chroma+resize" );
> +            es_format_Copy( &fmt_mid, &p_filter->fmt_in );
> +            video_format_TransformTo( &fmt_mid.video, p_filter->fmt_out.video.orientation );
> +        }
> +        else
> +        {
> +            /* Lets try resize+chroma first, then transform */
> +            msg_Dbg( p_filter, "Trying to build chroma+resize, then transform" );
> +            es_format_Copy( &fmt_mid, &p_filter->fmt_out );
> +            /* Set mid format to same rotation as source,
> +             * so we get proper scaling before transform */
> +            video_format_TransformTo( &fmt_mid.video, p_filter->fmt_in.video.orientation );
> +        }
>
> +        int i_ret = CreateChain( p_filter, &fmt_mid );
> +        es_format_Clean( &fmt_mid );
> +        if( i_ret == VLC_SUCCESS )
> +            return VLC_SUCCESS;
> +    }
>      return VLC_EGENERIC;
>  }
>
> --
> 2.21.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel