diff mbox series

[FFmpeg-devel,v2,4/5] libswscale: Enable hscale_avx2 for all input sizes.

Message ID 20220217100420.1113388-1-alankelly@google.com
State New
Headers show
Series [FFmpeg-devel,v2,1/5] libswscale: Check and propagate memory allocation errors from ff_shuffle_filter_coefficients. | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Alan Kelly Feb. 17, 2022, 10:04 a.m. UTC 
ff_shuffle_filter_coefficients shuffles the tail as required.
---
 libswscale/utils.c       | 19 ++++++++++++++++---
 libswscale/x86/swscale.c |  6 ++----
 2 files changed, 18 insertions(+), 7 deletions(-)

Comments

Alan Kelly April 22, 2022, 8:04 a.m. UTC | #1 
Ping!

On Thu, Feb 17, 2022 at 11:04 AM Alan Kelly <alankelly@google.com> wrote:

> ff_shuffle_filter_coefficients shuffles the tail as required.
> ---
>  libswscale/utils.c       | 19 ++++++++++++++++---
>  libswscale/x86/swscale.c |  6 ++----
>  2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index 7c8e1bbdde..d818c9ce55 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -285,8 +285,7 @@ int ff_shuffle_filter_coefficients(SwsContext *c, int
> *filterPos,
>  #if ARCH_X86_64
>      int i, j, k;
>      int cpu_flags = av_get_cpu_flags();
> -    // avx2 hscale filter processes 16 pixel blocks.
> -    if (!filter || dstW % 16 != 0)
> +    if (!filter)
>          return 0;
>      if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags &
> AV_CPU_FLAG_SLOW_GATHER)) {
>          if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
> @@ -298,9 +297,11 @@ int ff_shuffle_filter_coefficients(SwsContext *c, int
> *filterPos,
>             }
>             // Do not swap filterPos for pixels which won't be processed by
>             // the main loop.
> -           for (i = 0; i + 8 <= dstW; i += 8) {
> +           for (i = 0; i + 16 <= dstW; i += 16) {
>                 FFSWAP(int, filterPos[i + 2], filterPos[i + 4]);
>                 FFSWAP(int, filterPos[i + 3], filterPos[i + 5]);
> +               FFSWAP(int, filterPos[i + 10], filterPos[i + 12]);
> +               FFSWAP(int, filterPos[i + 11], filterPos[i + 13]);
>             }
>             if (filterSize > 4) {
>                 // 16 pixels are processed at a time.
> @@ -314,6 +315,18 @@ int ff_shuffle_filter_coefficients(SwsContext *c, int
> *filterPos,
>                         }
>                     }
>                 }
> +               // 4 pixels are processed at a time in the tail.
> +               for (; i < dstW; i += 4) {
> +                   // 4 filter coeffs are processed at a time.
> +                   int rem = dstW - i >= 4 ? 4 : dstW - i;
> +                   for (k = 0; k + 4 <= filterSize; k += 4) {
> +                       for (j = 0; j < rem; ++j) {
> +                           int from = (i + j) * filterSize + k;
> +                           int to = i * filterSize + j * 4 + k * 4;
> +                           memcpy(&filter[to], &filterCopy[from], 4 *
> sizeof(int16_t));
> +                       }
> +                   }
> +               }
>             }
>             av_free(filterCopy);
>          }
> diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> index 73869355b8..76f5a70fc5 100644
> --- a/libswscale/x86/swscale.c
> +++ b/libswscale/x86/swscale.c
> @@ -691,10 +691,8 @@ switch(c->dstBpc){ \
>
>      if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags &
> AV_CPU_FLAG_SLOW_GATHER)) {
>          if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
> -            if (c->chrDstW % 16 == 0)
> -                ASSIGN_AVX2_SCALE_FUNC(c->hcScale, c->hChrFilterSize);
> -            if (c->dstW % 16 == 0)
> -                ASSIGN_AVX2_SCALE_FUNC(c->hyScale, c->hLumFilterSize);
> +            ASSIGN_AVX2_SCALE_FUNC(c->hcScale, c->hChrFilterSize);
> +            ASSIGN_AVX2_SCALE_FUNC(c->hyScale, c->hLumFilterSize);
>          }
>      }
>
> --
> 2.35.1.265.g69c8d7142f-goog
>
>
Alan Kelly July 13, 2022, 8:05 a.m. UTC | #2 
Pushing this back up to the top. This is required to enable the previous
patch in this chain. Thanks

On Fri, Apr 22, 2022 at 10:04 AM Alan Kelly <alankelly@google.com> wrote:

> Ping!
>
> On Thu, Feb 17, 2022 at 11:04 AM Alan Kelly <alankelly@google.com> wrote:
>
>> ff_shuffle_filter_coefficients shuffles the tail as required.
>> ---
>>  libswscale/utils.c       | 19 ++++++++++++++++---
>>  libswscale/x86/swscale.c |  6 ++----
>>  2 files changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/libswscale/utils.c b/libswscale/utils.c
>> index 7c8e1bbdde..d818c9ce55 100644
>> --- a/libswscale/utils.c
>> +++ b/libswscale/utils.c
>> @@ -285,8 +285,7 @@ int ff_shuffle_filter_coefficients(SwsContext *c, int
>> *filterPos,
>>  #if ARCH_X86_64
>>      int i, j, k;
>>      int cpu_flags = av_get_cpu_flags();
>> -    // avx2 hscale filter processes 16 pixel blocks.
>> -    if (!filter || dstW % 16 != 0)
>> +    if (!filter)
>>          return 0;
>>      if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags &
>> AV_CPU_FLAG_SLOW_GATHER)) {
>>          if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
>> @@ -298,9 +297,11 @@ int ff_shuffle_filter_coefficients(SwsContext *c,
>> int *filterPos,
>>             }
>>             // Do not swap filterPos for pixels which won't be processed
>> by
>>             // the main loop.
>> -           for (i = 0; i + 8 <= dstW; i += 8) {
>> +           for (i = 0; i + 16 <= dstW; i += 16) {
>>                 FFSWAP(int, filterPos[i + 2], filterPos[i + 4]);
>>                 FFSWAP(int, filterPos[i + 3], filterPos[i + 5]);
>> +               FFSWAP(int, filterPos[i + 10], filterPos[i + 12]);
>> +               FFSWAP(int, filterPos[i + 11], filterPos[i + 13]);
>>             }
>>             if (filterSize > 4) {
>>                 // 16 pixels are processed at a time.
>> @@ -314,6 +315,18 @@ int ff_shuffle_filter_coefficients(SwsContext *c,
>> int *filterPos,
>>                         }
>>                     }
>>                 }
>> +               // 4 pixels are processed at a time in the tail.
>> +               for (; i < dstW; i += 4) {
>> +                   // 4 filter coeffs are processed at a time.
>> +                   int rem = dstW - i >= 4 ? 4 : dstW - i;
>> +                   for (k = 0; k + 4 <= filterSize; k += 4) {
>> +                       for (j = 0; j < rem; ++j) {
>> +                           int from = (i + j) * filterSize + k;
>> +                           int to = i * filterSize + j * 4 + k * 4;
>> +                           memcpy(&filter[to], &filterCopy[from], 4 *
>> sizeof(int16_t));
>> +                       }
>> +                   }
>> +               }
>>             }
>>             av_free(filterCopy);
>>          }
>> diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
>> index 73869355b8..76f5a70fc5 100644
>> --- a/libswscale/x86/swscale.c
>> +++ b/libswscale/x86/swscale.c
>> @@ -691,10 +691,8 @@ switch(c->dstBpc){ \
>>
>>      if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags &
>> AV_CPU_FLAG_SLOW_GATHER)) {
>>          if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
>> -            if (c->chrDstW % 16 == 0)
>> -                ASSIGN_AVX2_SCALE_FUNC(c->hcScale, c->hChrFilterSize);
>> -            if (c->dstW % 16 == 0)
>> -                ASSIGN_AVX2_SCALE_FUNC(c->hyScale, c->hLumFilterSize);
>> +            ASSIGN_AVX2_SCALE_FUNC(c->hcScale, c->hChrFilterSize);
>> +            ASSIGN_AVX2_SCALE_FUNC(c->hyScale, c->hLumFilterSize);
>>          }
>>      }
>>
>> --
>> 2.35.1.265.g69c8d7142f-goog
>>
>>
Michael Niedermayer July 14, 2022, 4:56 p.m. UTC | #3 
On Thu, Feb 17, 2022 at 11:04:20AM +0100, Alan Kelly wrote:
> ff_shuffle_filter_coefficients shuffles the tail as required.
> ---
>  libswscale/utils.c       | 19 ++++++++++++++++---
>  libswscale/x86/swscale.c |  6 ++----
>  2 files changed, 18 insertions(+), 7 deletions(-)

it seems patch 3 and 4 together fail fate with current git master
(i think 1 & 2 are already applied)

make V=2 fate-checkasm-sw_scale
TEST    checkasm-sw_scale
./tests/fate-run.sh fate-checkasm-sw_scale "fate-suite/" "" "ffmpeg" 'run tests/checkasm/checkasm --test=sw_scale' 'null' '' '' '1' '' '' '' '' '' '' '' '' '' ''
 ffmpeg/tests/checkasm/checkasm --test=sw_scale
Test checkasm-sw_scale failed. Look at tests/data/fate/checkasm-sw_scale.err for details.
checkasm: using random seed 1761410321
MMXEXT:
 - sw_scale.yuv2yuvX [OK]
SSE2:
 - sw_scale.hscale   [OK]
SSE3:
 - sw_scale.yuv2yuvX [OK]
SSSE3:
 - sw_scale.hscale   [OK]
SSE4.1:
 - sw_scale.hscale   [OK]
AVX2:
   hscale_8_to_15__fs_4_dstW_8_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_4_dstW_24_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_8_dstW_8_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_8_dstW_24_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_12_dstW_8_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_12_dstW_24_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_16_dstW_8_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_16_dstW_24_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_32_dstW_8_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_32_dstW_24_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_40_dstW_8_avx2 (sw_scale.c:235)
   hscale_8_to_15__fs_40_dstW_24_avx2 (sw_scale.c:235)
 - sw_scale.hscale   [FAILED]
 - sw_scale.yuv2yuvX [OK]
checkasm: 12 of 504 tests have failed
tests/Makefile:304: recipe for target 'fate-checkasm-sw_scale' failed
make: *** [fate-checkasm-sw_scale] Error 1





[...]
diff mbox series

Patch

diff --git a/libswscale/utils.c b/libswscale/utils.c
index 7c8e1bbdde..d818c9ce55 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -285,8 +285,7 @@  int ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos,
 #if ARCH_X86_64
     int i, j, k;
     int cpu_flags = av_get_cpu_flags();
-    // avx2 hscale filter processes 16 pixel blocks.
-    if (!filter || dstW % 16 != 0)
+    if (!filter)
         return 0;
     if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags & AV_CPU_FLAG_SLOW_GATHER)) {
         if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
@@ -298,9 +297,11 @@  int ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos,
            }
            // Do not swap filterPos for pixels which won't be processed by
            // the main loop.
-           for (i = 0; i + 8 <= dstW; i += 8) {
+           for (i = 0; i + 16 <= dstW; i += 16) {
                FFSWAP(int, filterPos[i + 2], filterPos[i + 4]);
                FFSWAP(int, filterPos[i + 3], filterPos[i + 5]);
+               FFSWAP(int, filterPos[i + 10], filterPos[i + 12]);
+               FFSWAP(int, filterPos[i + 11], filterPos[i + 13]);
            }
            if (filterSize > 4) {
                // 16 pixels are processed at a time.
@@ -314,6 +315,18 @@  int ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos,
                        }
                    }
                }
+               // 4 pixels are processed at a time in the tail.
+               for (; i < dstW; i += 4) {
+                   // 4 filter coeffs are processed at a time.
+                   int rem = dstW - i >= 4 ? 4 : dstW - i;
+                   for (k = 0; k + 4 <= filterSize; k += 4) {
+                       for (j = 0; j < rem; ++j) {
+                           int from = (i + j) * filterSize + k;
+                           int to = i * filterSize + j * 4 + k * 4;
+                           memcpy(&filter[to], &filterCopy[from], 4 * sizeof(int16_t));
+                       }
+                   }
+               }
            }
            av_free(filterCopy);
         }
diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
index 73869355b8..76f5a70fc5 100644
--- a/libswscale/x86/swscale.c
+++ b/libswscale/x86/swscale.c
@@ -691,10 +691,8 @@  switch(c->dstBpc){ \
 
     if (EXTERNAL_AVX2_FAST(cpu_flags) && !(cpu_flags & AV_CPU_FLAG_SLOW_GATHER)) {
         if ((c->srcBpc == 8) && (c->dstBpc <= 14)) {
-            if (c->chrDstW % 16 == 0)
-                ASSIGN_AVX2_SCALE_FUNC(c->hcScale, c->hChrFilterSize);
-            if (c->dstW % 16 == 0)
-                ASSIGN_AVX2_SCALE_FUNC(c->hyScale, c->hLumFilterSize);
+            ASSIGN_AVX2_SCALE_FUNC(c->hcScale, c->hChrFilterSize);
+            ASSIGN_AVX2_SCALE_FUNC(c->hyScale, c->hLumFilterSize);
         }
     }