Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/5] libswscale: Re-factor ff_shuffle_filter_coefficients.
@ 2022-02-09  9:09 Alan Kelly
  2022-02-16 21:21 ` Michael Niedermayer
  0 siblings, 1 reply; 2+ messages in thread
From: Alan Kelly @ 2022-02-09  9:09 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Alan Kelly

Make the code more readable and follow the style guide.
---
 libswscale/utils.c | 64 +++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/libswscale/utils.c b/libswscale/utils.c
index c5ea8853d5..1d919e863a 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -278,39 +278,49 @@ 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){
+void 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, j, k;
     int cpu_flags = av_get_cpu_flags();
+    // avx2 hscale filter processes 16 pixel blocks.
+    if (!filter || dstW % 16 != 0)
+        return;
     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;
+               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;
 }
 
 int sws_isSupportedInput(enum AVPixelFormat pix_fmt)
-- 
2.35.0.263.gb82422642f-goog

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/5] libswscale: Re-factor ff_shuffle_filter_coefficients.
  2022-02-09  9:09 [FFmpeg-devel] [PATCH 1/5] libswscale: Re-factor ff_shuffle_filter_coefficients Alan Kelly
@ 2022-02-16 21:21 ` Michael Niedermayer
  0 siblings, 0 replies; 2+ messages in thread
From: Michael Niedermayer @ 2022-02-16 21:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

On Wed, Feb 09, 2022 at 10:09:45AM +0100, Alan Kelly wrote:
> Make the code more readable and follow the style guide.
> ---
>  libswscale/utils.c | 64 +++++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index c5ea8853d5..1d919e863a 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -278,39 +278,49 @@ 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){
> +void 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, j, k;
>      int cpu_flags = av_get_cpu_flags();
> +    // avx2 hscale filter processes 16 pixel blocks.
> +    if (!filter || dstW % 16 != 0)
> +        return;
>      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;

this is not a matter of style, it changes a null pointer dereference on error
to simply returning with no error handling. I know the next patches
change this again but that makes it just more messy


> +               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);

The null check is unneeded


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

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri

[-- 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".

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-02-16 21:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09  9:09 [FFmpeg-devel] [PATCH 1/5] libswscale: Re-factor ff_shuffle_filter_coefficients Alan Kelly
2022-02-16 21:21 ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git