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

[Alan Kelly]

Hi,

Is anybody interested in this patch set?

Thanks!

On Mon, Jan 10, 2022, 15:58 Alan Kelly <alankelly at google.com> wrote:

> Make the code more readable, follow the style guide and propagate memory
> allocation errors.
> ---
>  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;
>      int cpu_flags = av_get_cpu_flags();
> +    if (!filter || dstW % 16 != 0) return 0;
>      if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags &
> AV_CPU_FLAG_SLOW_GATHER)) {
> -        if ((c->srcBpc == 8) && (c->dstBpc <= 14)){
> -            if (dstW % 16 == 0){
> -                if (filter != NULL){
> -                    for (i = 0; i < dstW; i += 8){
> -                        FFSWAP(int, filterPos[i + 2], filterPos[i+4]);
> -                        FFSWAP(int, filterPos[i + 3], filterPos[i+5]);
> -                    }
> -                    if (filterSize > 4){
> -                        int16_t *tmp2 = av_malloc(dstW * filterSize * 2);
> -                        memcpy(tmp2, filter, dstW * filterSize * 2);
> -                        for (i = 0; i < dstW; i += 16){//pixel
> -                            for (k = 0; k < filterSize / 4; ++k){//fcoeff
> -                                for (j = 0; j < 16; ++j){//inner pixel
> -                                    for (l = 0; l < 4; ++l){//coeff
> -                                        int from = i * filterSize + j *
> filterSize + k * 4 + l;
> -                                        int to = (i) * filterSize + j * 4
> + l + k * 64;
> -                                        filter[to] = tmp2[from];
> -                                    }
> -                                }
> -                            }
> -                        }
> -                        av_free(tmp2);
> -                    }
> -                }
> -            }
> +        if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
> +           int16_t *filterCopy = NULL;
> +           if (filterSize > 4) {
> +               if (!FF_ALLOC_TYPED_ARRAY(filterCopy, dstW * filterSize))
> +                   return AVERROR(ENOMEM);
> +               memcpy(filterCopy, filter, dstW * filterSize *
> sizeof(int16_t));
> +           }
> +           // Do not swap filterPos for pixels which won't be processed by
> +           // the main loop.
> +           for (i = 0; i + 8 <= dstW; i += 8) {
> +               FFSWAP(int, filterPos[i + 2], filterPos[i + 4]);
> +               FFSWAP(int, filterPos[i + 3], filterPos[i + 5]);
> +           }
> +           if (filterSize > 4) {
> +               // 16 pixels are processed at a time.
> +               for (i = 0; i + 16 <= dstW; i += 16) {
> +                   // 4 filter coeffs are processed at a time.
> +                   for (k = 0; k + 4 <= filterSize; k += 4) {
> +                       for (j = 0; j < 16; ++j) {
> +                           int from = (i + j) * filterSize + k;
> +                           int to = i * filterSize + j * 4 + k * 16;
> +                           memcpy(&filter[to], &filterCopy[from], 4 *
> sizeof(int16_t));
> +                       }
> +                   }
> +               }
> +           }
> +           if (filterCopy)
> +               av_free(filterCopy);
>          }
>      }
>  #endif
> +    return 0;
>  }
>
>  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;
>              if ((ret = initFilter(&c->hChrFilter, &c->hChrFilterPos,
>                             &c->hChrFilterSize, c->chrXInc,
>                             c->chrSrcW, c->chrDstW, filterAlign, 1 << 14,
> @@ -1846,7 +1855,8 @@ av_cold int sws_init_context(SwsContext *c,
> SwsFilter *srcFilter,
>                             get_local_pos(c, c->chrSrcHSubSample,
> c->src_h_chr_pos, 0),
>                             get_local_pos(c, c->chrDstHSubSample,
> c->dst_h_chr_pos, 0))) < 0)
>                  goto fail;
> -            ff_shuffle_filter_coefficients(c, c->hChrFilterPos,
> c->hChrFilterSize, c->hChrFilter, c->chrDstW);
> +            if ((ret = ff_shuffle_filter_coefficients(c,
> c->hChrFilterPos, c->hChrFilterSize, c->hChrFilter, c->chrDstW)) != 0)
> +                goto nomem;
>          }
>      } // initialize horizontal stuff
>
> --
> 2.34.1.575.g55b058a8bb-goog
>
>