[FFmpeg-devel] [PATCH 1/4] avformat/asfcrypt: Fix wrong array length in function declaration

[FFmpeg-devel] [PATCH 1/4] avformat/asfcrypt: Fix wrong array length in function declaration

[Andreas Rheinhardt]

multiswap_step() and multiswap_inv_step() both only require
six keys; in all current callers, these keys are part of
an array of twelve keys, yet in some of these callers the keys
given to these functions point to the second half of these
twelve keys, so that only six keys are available to these functions.
This led to -Wstringop-overread warnings when compiling with GCC 12.1.
Fix these by adapting the declaration of these functions.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/asfcrypt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/asfcrypt.c b/libavformat/asfcrypt.c
index c77e37503e..ed68fb60ed 100644
--- a/libavformat/asfcrypt.c
+++ b/libavformat/asfcrypt.c
@@ -73,7 +73,7 @@ static void multiswap_invert_keys(uint32_t keys[12])
         keys[i] = inverse(keys[i]);
 }
 
-static uint32_t multiswap_step(const uint32_t keys[12], uint32_t v)
+static uint32_t multiswap_step(const uint32_t keys[6], uint32_t v)
 {
     int i;
     v *= keys[0];
@@ -85,7 +85,7 @@ static uint32_t multiswap_step(const uint32_t keys[12], uint32_t v)
     return v;
 }
 
-static uint32_t multiswap_inv_step(const uint32_t keys[12], uint32_t v)
+static uint32_t multiswap_inv_step(const uint32_t keys[6], uint32_t v)
 {
     int i;
     v -= keys[5];
-- 
2.34.1

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

[FFmpeg-devel] [PATCH 2/4] avcodec/snowenc: Don't pass int[2] as parameter declared as int[3]

[Andreas Rheinhardt]

check_block_inter() currently does this when calling check_block().
This leads to a -Wstringop-overflow= warning when compiling with
GCC 12.1.

Given that the main part of the body of check_block() consists
of an "if (intra) { ... } else { ... }" which is true iff
check_block() is not called from check_block_inter(),
it makes sense to fix this by just inlining check_block()
check_block_inter() and turning check_block() into a new
check_block_intra() (with the inter parts removed, of course).

This should also not make much of a difference for the generated code
given that both check_block() as well as check_block_inter()
are already marked as av_always_inline, so this commit follows
this route to fix the issue.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/snowenc.c | 64 +++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 24 deletions(-)

diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
index 207948675b..5e6b489c43 100644
--- a/libavcodec/snowenc.c
+++ b/libavcodec/snowenc.c
@@ -892,34 +892,23 @@ static int encode_subband(SnowContext *s, SubBand *b, const IDWTELEM *src, const
 //    encode_subband_dzr(s, b, src, parent, stride, orientation);
 }
 
-static av_always_inline int check_block(SnowContext *s, int mb_x, int mb_y, int p[3], int intra, uint8_t (*obmc_edged)[MB_SIZE * 2], int *best_rd){
+static av_always_inline int check_block_intra(SnowContext *s, int mb_x, int mb_y, int p[3],
+                                              uint8_t (*obmc_edged)[MB_SIZE * 2], int *best_rd)
+{
     const int b_stride= s->b_width << s->block_max_depth;
     BlockNode *block= &s->block[mb_x + mb_y * b_stride];
     BlockNode backup= *block;
-    unsigned value;
-    int rd, index;
+    int rd;
 
     av_assert2(mb_x>=0 && mb_y>=0);
     av_assert2(mb_x<b_stride);
 
-    if(intra){
-        block->color[0] = p[0];
-        block->color[1] = p[1];
-        block->color[2] = p[2];
-        block->type |= BLOCK_INTRA;
-    }else{
-        index= (p[0] + 31*p[1]) & (ME_CACHE_SIZE-1);
-        value= s->me_cache_generation + (p[0]>>10) + (p[1]<<6) + (block->ref<<12);
-        if(s->me_cache[index] == value)
-            return 0;
-        s->me_cache[index]= value;
-
-        block->mx= p[0];
-        block->my= p[1];
-        block->type &= ~BLOCK_INTRA;
-    }
+    block->color[0] = p[0];
+    block->color[1] = p[1];
+    block->color[2] = p[2];
+    block->type |= BLOCK_INTRA;
 
-    rd= get_block_rd(s, mb_x, mb_y, 0, obmc_edged) + s->intra_penalty * !!intra;
+    rd = get_block_rd(s, mb_x, mb_y, 0, obmc_edged) + s->intra_penalty;
 
 //FIXME chroma
     if(rd < *best_rd){
@@ -934,8 +923,35 @@ static av_always_inline int check_block(SnowContext *s, int mb_x, int mb_y, int
 /* special case for int[2] args we discard afterwards,
  * fixes compilation problem with gcc 2.95 */
 static av_always_inline int check_block_inter(SnowContext *s, int mb_x, int mb_y, int p0, int p1, uint8_t (*obmc_edged)[MB_SIZE * 2], int *best_rd){
-    int p[2] = {p0, p1};
-    return check_block(s, mb_x, mb_y, p, 0, obmc_edged, best_rd);
+    const int b_stride = s->b_width << s->block_max_depth;
+    BlockNode *block = &s->block[mb_x + mb_y * b_stride];
+    BlockNode backup = *block;
+    unsigned value;
+    int rd, index;
+
+    av_assert2(mb_x >= 0 && mb_y >= 0);
+    av_assert2(mb_x < b_stride);
+
+    index = (p0 + 31 * p1) & (ME_CACHE_SIZE-1);
+    value = s->me_cache_generation + (p0 >> 10) + (p1 << 6) + (block->ref << 12);
+    if (s->me_cache[index] == value)
+        return 0;
+    s->me_cache[index] = value;
+
+    block->mx = p0;
+    block->my = p1;
+    block->type &= ~BLOCK_INTRA;
+
+    rd = get_block_rd(s, mb_x, mb_y, 0, obmc_edged);
+
+//FIXME chroma
+    if (rd < *best_rd) {
+        *best_rd = rd;
+        return 1;
+    } else {
+        *block   = backup;
+        return 0;
+    }
 }
 
 static av_always_inline int check_4block_inter(SnowContext *s, int mb_x, int mb_y, int p0, int p1, int ref, int *best_rd){
@@ -1092,7 +1108,7 @@ static void iterative_me(SnowContext *s){
                 // get previous score (cannot be cached due to OBMC)
                 if(pass > 0 && (block->type&BLOCK_INTRA)){
                     int color0[3]= {block->color[0], block->color[1], block->color[2]};
-                    check_block(s, mb_x, mb_y, color0, 1, obmc_edged, &best_rd);
+                    check_block_intra(s, mb_x, mb_y, color0, obmc_edged, &best_rd);
                 }else
                     check_block_inter(s, mb_x, mb_y, block->mx, block->my, obmc_edged, &best_rd);
 
@@ -1150,7 +1166,7 @@ static void iterative_me(SnowContext *s){
                 }
                 best_rd= ref_rd;
                 *block= ref_b;
-                check_block(s, mb_x, mb_y, color, 1, obmc_edged, &best_rd);
+                check_block_intra(s, mb_x, mb_y, color, obmc_edged, &best_rd);
                 //FIXME RD style color selection
                 if(!same_block(block, &backup)){
                     if(tb ) tb ->type &= ~BLOCK_OPT;
-- 
2.34.1

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

[FFmpeg-devel] [PATCH 3/4] avcodec/h264_loopfilter: Fix incorrect function parameter array size

[Andreas Rheinhardt]

filter_mb_mbaff_edgev() and filter_mb_mbaff_edgecv()
have a function parameter whose expected size depends upon
another parameter: It is 2 * bsi + 1 (with bsi always being 1 or 2).
This array is declared as const int16_t[7], yet some of the callers
with bsi == 1 call it with only an const int16_t[4] available.
This leads to -Wstringop-overread warnings from GCC 12.1.

This commit fixes these by replacing [7] with [/* 2 * bsi + 1 */],
so that the expected range and its dependence on bsi is immediately
visible.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/h264_loopfilter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h264_loopfilter.c b/libavcodec/h264_loopfilter.c
index 2440cfa831..c164a289b7 100644
--- a/libavcodec/h264_loopfilter.c
+++ b/libavcodec/h264_loopfilter.c
@@ -143,7 +143,7 @@ static av_always_inline void filter_mb_edgecv(uint8_t *pix, int stride,
 
 static av_always_inline void filter_mb_mbaff_edgev(const H264Context *h, uint8_t *pix,
                                                    int stride,
-                                                   const int16_t bS[7], int bsi,
+                                                   const int16_t bS[ /* 1 + 2 * bsi */ ], int bsi,
                                                    int qp, int a, int b,
                                                    int intra)
 {
@@ -166,7 +166,7 @@ static av_always_inline void filter_mb_mbaff_edgev(const H264Context *h, uint8_t
 
 static av_always_inline void filter_mb_mbaff_edgecv(const H264Context *h,
                                                     uint8_t *pix, int stride,
-                                                    const int16_t bS[7],
+                                                    const int16_t bS[ /* 1 + 2 * bsi */ ],
                                                     int bsi, int qp, int a,
                                                     int b, int intra)
 {
-- 
2.34.1

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

[FFmpeg-devel] [PATCH 4/4] avcodec/svq1enc: Use unsigned for parameter >= 0 to workaround GCC bug

[Andreas Rheinhardt]

encode_block() in svq1enc.c looks like the following:

static int encode_block(int block[7][256], int level)
{
    int best_score = 0;

    for (unsigned x = 0; x < level; x++) {
        int v = block[1][x];
        block[level][x] = 0;
        best_score      += v * v;
    }

    if (level > 0 && best_score > 64) {
        int score = 0;

        score += encode_block(block, level - 1);
        score += encode_block(block, level - 1);

        if (score < best_score) {
            best_score = score;
        }
    }

    return best_score;
}

When called from outside of encode_block(), it is always called with
level == 5.

This triggers a bug [1] in GCC: On -O3, it creates eight clones of
encode_block with different values of level inlined into it. The clones
with negative values are of course useless*, but they also lead to
-Warray-bounds warnings, because they access block[-1].

This has been mitigated in GCC 12: It no longer creates clones
for parameters that it knows are impossible. Somehow switching levels
to unsigned makes GCC know this. Therefore this commit does this.
(For GCC 11, this changes the warning to "array subscript 4294967295 is
above array bounds" from "array subscript -1 is below array bounds".)

[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102513

*: These clones can actually be discarded when compiling with
-ffunction-sections.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/svq1enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 3c2d594632..6072f8d07d 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -91,7 +91,7 @@ static int ssd_int8_vs_int16_c(const int8_t *pix1, const int16_t *pix2,
 }
 
 static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
-                        uint8_t *decoded, int stride, int level,
+                        uint8_t *decoded, int stride, unsigned level,
                         int threshold, int lambda, int intra)
 {
     int count, y, x, i, j, split, best_mean, best_score, best_count;
-- 
2.34.1

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