Re: [FFmpeg-devel] [PATCH 1/4] libswscale: Re-factor ff_shuffle_filter_coefficients.

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2784 bytes --]

On Mon, Jan 10, 2022 at 03:58:33PM +0100, Alan Kelly wrote:
> Make the code more readable, follow the style guide and propagate memory
> allocation errors.

Cosmetics and bugfixes should not be in the same patch


> ---
>  libswscale/swscale_internal.h |  2 +-
>  libswscale/utils.c            | 68 ++++++++++++++++++++---------------
>  2 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> index 3a78d95ba6..26d28d42e6 100644
> --- a/libswscale/swscale_internal.h
> +++ b/libswscale/swscale_internal.h
> @@ -1144,5 +1144,5 @@ void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
>  #define MAX_LINES_AHEAD 4
>  
>  //shuffle filter and filterPos for hyScale and hcScale filters in avx2
> -void ff_shuffle_filter_coefficients(SwsContext *c, int* filterPos, int filterSize, int16_t *filter, int dstW);
> +int ff_shuffle_filter_coefficients(SwsContext *c, int* filterPos, int filterSize, int16_t *filter, int dstW);
>  #endif /* SWSCALE_SWSCALE_INTERNAL_H */
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index c5ea8853d5..52f07e1661 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -278,39 +278,47 @@ static const FormatEntry format_entries[] = {
>      [AV_PIX_FMT_P416LE]      = { 1, 1 },
>  };
>  
> -void ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos, int filterSize, int16_t *filter, int dstW){
> +int ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos,
> +                                   int filterSize, int16_t *filter,
> +                                   int dstW)
> +{
>  #if ARCH_X86_64

> -    int i, j, k, l;
> +    int i = 0, j = 0, k = 0;

why?
they are set when used if iam not mistaken


>      int cpu_flags = av_get_cpu_flags();

> +    if (!filter || dstW % 16 != 0) return 0;

please add \n also a comment what the dstW & 16 case exactly does and why


[...]
>  int sws_isSupportedInput(enum AVPixelFormat pix_fmt)
> @@ -1836,7 +1844,8 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter,
>                             get_local_pos(c, 0, 0, 0),
>                             get_local_pos(c, 0, 0, 0))) < 0)
>                  goto fail;
> -            ff_shuffle_filter_coefficients(c, c->hLumFilterPos, c->hLumFilterSize, c->hLumFilter, dstW);
> +            if ((ret = ff_shuffle_filter_coefficients(c, c->hLumFilterPos, c->hLumFilterSize, c->hLumFilter, dstW)) != 0)
> +                goto nomem;

This is confusing as ret is never used, also error codes are <0

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".