diff mbox series

[FFmpeg-devel] swscale: fix style issues and check malloc in ff_shuffle_filter_coefficients

Message ID MrTIbHq--3-2@lynne.ee
State New
Headers show
Series [FFmpeg-devel] swscale: fix style issues and check malloc in ff_shuffle_filter_coefficients | expand

Checks

Context Check Description
andriy/make_x86 fail Make failed
andriy/configureppc warning Failed to apply patch

Commit Message

Lynne Dec. 21, 2021, 6:43 p.m. UTC 
Google's net worth for 2021:                       1 000 000 000 000 USD.
Google's revenue for 2021:                           204 000 000 000 USD.
Average annual salary for a Google engineer:                 135 000 USD.

The greatest, most powerful beacon of capitalism on the planet sends
a patch to an open-source project ran by volunteers.
Said monument of wealth's revenue is dependent upon said project ran
by volunteers.

Issues per line of code: ~2.4.

Guess which project's ran by volunteers who receive nothing from Google
yet have to fix issues in patches meant to fix issues?

Patch attached without snarky commit message. But I hope my point gets across.
Subject: [PATCH] swscale: fix style issues and check malloc in
 ff_shuffle_filter_coefficients

---
 libswscale/swscale_internal.h |  2 +-
 libswscale/utils.c            | 65 +++++++++++++++++++++--------------
 2 files changed, 40 insertions(+), 27 deletions(-)

Comments

Andreas Rheinhardt Dec. 21, 2021, 7:18 p.m. UTC | #1 
Lynne:
> 
> +        if (filterSize > 4) {
> +            int16_t *tmp2 = av_malloc(dstW * filterSize * 2);
> +            if (!tmp2)
> +                return AVERROR(ENOMEM);
> +
> +            memcpy(tmp2, filter, dstW * filterSize * 2);

Could use av_memdup().

- Andreas
Lynne Dec. 21, 2021, 8:53 p.m. UTC | #2 
21 Dec 2021, 20:18 by andreas.rheinhardt@outlook.com:

> Lynne:
>
>>
>> +        if (filterSize > 4) {
>> +            int16_t *tmp2 = av_malloc(dstW * filterSize * 2);
>> +            if (!tmp2)
>> +                return AVERROR(ENOMEM);
>> +
>> +            memcpy(tmp2, filter, dstW * filterSize * 2);
>>
>
> Could use av_memdup().
>

v2 attached, also fixed lack of return value on 32-bit systems
diff mbox series

Patch

diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index 64aa0b9804..e834f2aa91 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -1106,5 +1106,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 d4a72d3ce1..fbc1dc98ff 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -278,36 +278,40 @@  static const FormatEntry format_entries[] = {
     [AV_PIX_FMT_P416LE]      = { 1, 0 },
 };
 
-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 cpu_flags = av_get_cpu_flags();
-    if (EXTERNAL_AVX2_FAST(cpu_flags)){
-        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];
-                                    }
-                                }
-                            }
+
+    if (!EXTERNAL_AVX2_FAST(cpu_flags))
+        return 0;
+
+    if ((c->srcBpc == 8) && (c->dstBpc <= 14) && !(dstW % 16) && filter) {
+        for (int 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);
+            if (!tmp2)
+                return AVERROR(ENOMEM);
+
+            memcpy(tmp2, filter, dstW * filterSize * 2);
+
+            for (int i = 0; i < dstW; i += 16) {             //pixel
+                for (int k = 0; k < filterSize / 4; k++) {   //fcoeff
+                    for (int j = 0; j < 16; j++) {           //inner pixel
+                        for (int 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);
                     }
                 }
             }
+            av_free(tmp2);
         }
     }
 #endif
@@ -1836,7 +1840,12 @@  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)))
+                goto fail;
+
             if ((ret = initFilter(&c->hChrFilter, &c->hChrFilterPos,
                            &c->hChrFilterSize, c->chrXInc,
                            c->chrSrcW, c->chrDstW, filterAlign, 1 << 14,
@@ -1846,7 +1855,11 @@  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)))
+                goto fail;
         }
     } // initialize horizontal stuff