[FFmpeg-devel] [PATCH 1/7] avcodec/utils: apply the same alignment to YUV410 as we do to YUV420 when motion estimation is used

[FFmpeg-devel] [PATCH 1/7] avcodec/utils: apply the same alignment to YUV410 as we do to YUV420 when motion estimation is used

[Michael Niedermayer]

The snow encoder uses block based motion estimation which can read out of array if
insufficient alignment is used

Fixes: out of array access
Fixes: 68963/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-4979988435632128
Fixes: 68969/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-6239933667803136.fuzz

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/utils.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 337c00e789a..7914f799041 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -259,6 +259,9 @@ void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
         if (s->codec_id == AV_CODEC_ID_SVQ1) {
             w_align = 64;
             h_align = 64;
+        } else if (s->codec_id == AV_CODEC_ID_SNOW) {
+            w_align = 16;
+            h_align = 16;
         }
         break;
     case AV_PIX_FMT_RGB555:
-- 
2.45.2

_______________________________________________
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/7] avcodec/ratecontrol: Try to keep fps as a rational

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/ratecontrol.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
index 609d47faeb4..df27639ca73 100644
--- a/libavcodec/ratecontrol.c
+++ b/libavcodec/ratecontrol.c
@@ -56,20 +56,25 @@ void ff_write_pass1_stats(MpegEncContext *s)
              s->header_bits);
 }
 
-static double get_fps(AVCodecContext *avctx)
+static AVRational get_fpsQ(AVCodecContext *avctx)
 {
     if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
-        return av_q2d(avctx->framerate);
+        return avctx->framerate;
 
 FF_DISABLE_DEPRECATION_WARNINGS
-    return 1.0 / av_q2d(avctx->time_base)
 #if FF_API_TICKS_PER_FRAME
-        / FFMAX(avctx->ticks_per_frame, 1)
+    return av_div_q((AVRational){1, FFMAX(avctx->ticks_per_frame, 1)}, avctx->time_base);
+#else
+    return av_inv_q(avctx->time_base);
 #endif
-        ;
 FF_ENABLE_DEPRECATION_WARNINGS
 }
 
+static double get_fps(AVCodecContext *avctx)
+{
+    return av_q2d(get_fpsQ(avctx));
+}
+
 static inline double qp2bits(const RateControlEntry *rce, double qp)
 {
     if (qp <= 0.0) {
@@ -332,12 +337,13 @@ static int init_pass2(MpegEncContext *s)
     RateControlContext *rcc = &s->rc_context;
     AVCodecContext *a       = s->avctx;
     int i, toobig;
-    double fps             = get_fps(s->avctx);
+    AVRational fps         = get_fpsQ(s->avctx);
     double complexity[5]   = { 0 }; // approximate bits at quant=1
     uint64_t const_bits[5] = { 0 }; // quantizer independent bits
     uint64_t all_const_bits;
-    uint64_t all_available_bits = (uint64_t)(s->bit_rate *
-                                             (double)rcc->num_entries / fps);
+    uint64_t all_available_bits = av_rescale_q(s->bit_rate,
+                                               (AVRational){rcc->num_entries,1},
+                                               fps);
     double rate_factor          = 0;
     double step;
     const int filter_size = (int)(a->qblur * 4) | 1;
-- 
2.45.2

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

Re: [FFmpeg-devel] [PATCH 2/7] avcodec/ratecontrol: Try to keep fps as a rational

[Michael Niedermayer]

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

On Tue, Jun 18, 2024 at 03:48:21PM +0200, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/ratecontrol.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)

will apply patches 2,4,5

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

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

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

[FFmpeg-devel] [PATCH 3/7] avcodec/ratecontrol: Handle wanted bits overflow

[Michael Niedermayer]

Fixes: 5.92611e+20 is outside the range of representable values of type 'unsigned long'
Fixes: 68984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-5155755073273856

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/ratecontrol.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
index df27639ca73..86ec7a3443e 100644
--- a/libavcodec/ratecontrol.c
+++ b/libavcodec/ratecontrol.c
@@ -936,6 +936,7 @@ float ff_rate_estimate_qscale(MpegEncContext *s, int dry_run)
         wanted_bits = rce->expected_bits;
     } else {
         const MPVPicture *dts_pic;
+        double wanted_bits_double;
         rce = &local_rce;
 
         /* FIXME add a dts field to AVFrame and ensure it is set and use it
@@ -947,9 +948,14 @@ float ff_rate_estimate_qscale(MpegEncContext *s, int dry_run)
             dts_pic = s->last_pic.ptr;
 
         if (!dts_pic || dts_pic->f->pts == AV_NOPTS_VALUE)
-            wanted_bits = (uint64_t)(s->bit_rate * (double)picture_number / fps);
+            wanted_bits_double = s->bit_rate * (double)picture_number / fps;
         else
-            wanted_bits = (uint64_t)(s->bit_rate * (double)dts_pic->f->pts / fps);
+            wanted_bits_double = s->bit_rate * (double)dts_pic->f->pts / fps;
+        if (wanted_bits_double > INT64_MAX) {
+            av_log(s, AV_LOG_WARNING, "Bits exceed 64bit range\n");
+            wanted_bits = INT64_MAX;
+        } else
+            wanted_bits = (int64_t)wanted_bits_double;
     }
 
     diff = s->total_bits - wanted_bits;
-- 
2.45.2

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

Re: [FFmpeg-devel] [PATCH 3/7] avcodec/ratecontrol: Handle wanted bits overflow

[Michael Niedermayer]

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

On Tue, Jun 18, 2024 at 03:48:22PM +0200, Michael Niedermayer wrote:
> Fixes: 5.92611e+20 is outside the range of representable values of type 'unsigned long'
> Fixes: 68984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-5155755073273856
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/ratecontrol.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

will apply patch 3 and 6

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf

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

[FFmpeg-devel] [PATCH 4/7] avcodec/snowenc: MV limits due to mv_penalty table size

[Michael Niedermayer]

Fixes: out of array read
Fixes: 69673/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-5476592894148608

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/snowenc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
index 8d6dabae658..dd6ce36aa54 100644
--- a/libavcodec/snowenc.c
+++ b/libavcodec/snowenc.c
@@ -413,6 +413,7 @@ static int encode_q_branch(SnowEncContext *enc, int level, int x, int y)
     int my_context= av_log2(2*FFABS(left->my - top->my));
     int s_context= 2*left->level + 2*top->level + tl->level + tr->level;
     int ref, best_ref, ref_score, ref_mx, ref_my;
+    int range = MAX_MV >> (1 + qpel);
 
     av_assert0(sizeof(s->block_state) >= 256);
     if(s->keyframe){
@@ -454,6 +455,11 @@ static int encode_q_branch(SnowEncContext *enc, int level, int x, int y)
     c->xmax = - (x+1)*block_w + (w<<(LOG2_MB_SIZE - s->block_max_depth)) + 16-3;
     c->ymax = - (y+1)*block_w + (h<<(LOG2_MB_SIZE - s->block_max_depth)) + 16-3;
 
+    c->xmin = FFMAX(c->xmin,-range);
+    c->xmax = FFMIN(c->xmax, range);
+    c->ymin = FFMAX(c->ymin,-range);
+    c->ymax = FFMIN(c->ymax, range);
+
     if(P_LEFT[0]     > (c->xmax<<shift)) P_LEFT[0]    = (c->xmax<<shift);
     if(P_LEFT[1]     > (c->ymax<<shift)) P_LEFT[1]    = (c->ymax<<shift);
     if(P_TOP[0]      > (c->xmax<<shift)) P_TOP[0]     = (c->xmax<<shift);
-- 
2.45.2

_______________________________________________
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 5/7] avcodec/jfdctint_template: Fewer integer anomalies

[Michael Niedermayer]

Fixes: signed integer overflow: 105788 * -20995 cannot be represented in type 'int'
Fixes: signed integer overflow: 923211729 + 2073948236 cannot be represented in type 'int'
Fixes: signed integer overflow: 1281179284 + 2073948236 cannot be represented in type 'int'
Fixes: 68975/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PRORES_fuzzer-6266769177116672
Fixes: 68997/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PRORES_KS_fuzzer-6284237161431040

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/jfdctint_template.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/jfdctint_template.c b/libavcodec/jfdctint_template.c
index ca17300c324..aa2680132ee 100644
--- a/libavcodec/jfdctint_template.c
+++ b/libavcodec/jfdctint_template.c
@@ -69,7 +69,7 @@
 #define GLOBAL(x) x
 #define RIGHT_SHIFT(x, n) ((x) >> (n))
 #define MULTIPLY16C16(var,const) ((var)*(const))
-#define DESCALE(x,n)  RIGHT_SHIFT((x) + (1 << ((n) - 1)), n)
+#define DESCALE(x,n)  RIGHT_SHIFT((int)(x) + (1 << ((n) - 1)), n)
 
 
 /*
@@ -175,7 +175,7 @@
 #if BITS_IN_JSAMPLE == 8 && CONST_BITS<=13 && PASS1_BITS<=2
 #define MULTIPLY(var,const)  MULTIPLY16C16(var,const)
 #else
-#define MULTIPLY(var,const)  ((var) * (const))
+#define MULTIPLY(var,const)  (int)((var) * (unsigned)(const))
 #endif
 
 
@@ -261,7 +261,7 @@ FUNC(ff_jpeg_fdct_islow)(int16_t *data)
 {
   int tmp0, tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7;
   int tmp10, tmp11, tmp12, tmp13;
-  int z1, z2, z3, z4, z5;
+  unsigned z1, z2, z3, z4, z5;
   int16_t *dataptr;
   int ctr;
 
-- 
2.45.2

_______________________________________________
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 6/7] avcodec/vc2enc: Fix overflows with storing large values

[Michael Niedermayer]

Fixes: left shift of 1431634944 by 2 places cannot be represented in type 'int'
Fixes: left shift of 1073741824 by 1 places cannot be represented in type 'int'
Fixes: 69061/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC2_fuzzer-6325700826038272

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vc2enc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index 4ea836d9c9a..7fa6ddc4ca0 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -189,7 +189,9 @@ typedef struct VC2EncContext {
 static av_always_inline void put_vc2_ue_uint(PutBitContext *pb, uint32_t val)
 {
     int i;
-    int pbits = 0, bits = 0, topbit = 1, maxval = 1;
+    int bits = 0;
+    unsigned topbit = 1, maxval = 1;
+    uint64_t pbits = 0;
 
     if (!val++) {
         put_bits(pb, 1, 1);
@@ -206,12 +208,13 @@ static av_always_inline void put_vc2_ue_uint(PutBitContext *pb, uint32_t val)
 
     for (i = 0; i < bits; i++) {
         topbit >>= 1;
+        av_assert2(pbits <= UINT64_MAX>>3);
         pbits <<= 2;
         if (val & topbit)
             pbits |= 0x1;
     }
 
-    put_bits(pb, bits*2 + 1, (pbits << 1) | 1);
+    put_bits64(pb, bits*2 + 1, (pbits << 1) | 1);
 }
 
 static av_always_inline int count_vc2_ue_uint(uint32_t val)
-- 
2.45.2

_______________________________________________
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 7/7] avcodec/proresenc_kostya: use unsigned alpha for rotation

[Michael Niedermayer]

Fixes: left shift of negative value -208
Fixes: 69073/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PRORES_KS_fuzzer-4745020002336768

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/proresenc_kostya.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index 8b91ca1a98a..fe8cc5f0fda 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -343,7 +343,7 @@ static void get_slice_data(ProresContext *ctx, const uint16_t *src,
 
 static void get_alpha_data(ProresContext *ctx, const uint16_t *src,
                            ptrdiff_t linesize, int x, int y, int w, int h,
-                           int16_t *blocks, int mbs_per_slice, int abits)
+                           uint16_t *blocks, int mbs_per_slice, int abits)
 {
     const int slice_width = 16 * mbs_per_slice;
     int i, j, copy_w, copy_h;
-- 
2.45.2

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

Re: [FFmpeg-devel] [PATCH 7/7] avcodec/proresenc_kostya: use unsigned alpha for rotation

[Michael Niedermayer]

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

On Tue, Jun 18, 2024 at 03:48:26PM +0200, Michael Niedermayer wrote:
> Fixes: left shift of negative value -208
> Fixes: 69073/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PRORES_KS_fuzzer-4745020002336768
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/proresenc_kostya.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

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

Re: [FFmpeg-devel] [PATCH 1/7] avcodec/utils: apply the same alignment to YUV410 as we do to YUV420 when motion estimation is used

[Andreas Rheinhardt]

Michael Niedermayer:
> The snow encoder uses block based motion estimation which can read out of array if
> insufficient alignment is used
> 
> Fixes: out of array access
> Fixes: 68963/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-4979988435632128
> Fixes: 68969/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-6239933667803136.fuzz
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/utils.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 337c00e789a..7914f799041 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -259,6 +259,9 @@ void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
>          if (s->codec_id == AV_CODEC_ID_SVQ1) {
>              w_align = 64;
>              h_align = 64;
> +        } else if (s->codec_id == AV_CODEC_ID_SNOW) {
> +            w_align = 16;
> +            h_align = 16;
>          }
>          break;
>      case AV_PIX_FMT_RGB555:

avcodec_align_dimensions2() is for decoders and happens to be used by
ff_encode_alloc_frame(), too. But decoders should not be required to add
more padding because the decoder needs more. Instead the encoder should
add more padding itself (by using more than 2 * EDGE_WIDTH).

- Andreas

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

Re: [FFmpeg-devel] [PATCH 1/7] avcodec/utils: apply the same alignment to YUV410 as we do to YUV420 when motion estimation is used

[Michael Niedermayer]

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

On Tue, Jun 18, 2024 at 07:28:18PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > The snow encoder uses block based motion estimation which can read out of array if
> > insufficient alignment is used
> > 
> > Fixes: out of array access
> > Fixes: 68963/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-4979988435632128
> > Fixes: 68969/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-6239933667803136.fuzz
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/utils.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index 337c00e789a..7914f799041 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -259,6 +259,9 @@ void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
> >          if (s->codec_id == AV_CODEC_ID_SVQ1) {
> >              w_align = 64;
> >              h_align = 64;
> > +        } else if (s->codec_id == AV_CODEC_ID_SNOW) {
> > +            w_align = 16;
> > +            h_align = 16;
> >          }
> >          break;
> >      case AV_PIX_FMT_RGB555:
> 
> avcodec_align_dimensions2() is for decoders and happens to be used by
> ff_encode_alloc_frame(), too. But decoders should not be required to add
> more padding because the decoder needs more. Instead the encoder should
> add more padding itself (by using more than 2 * EDGE_WIDTH).

If you have verified that the decoder alignment is sufficient and 4:1:0 does not
need the alignment that 4:2:0 has for snow, then yes i can mess with some
EDGE_WIDTH uses in snow to fix the encoder crash

Again the snow decoder for 4:2:0 has 16x16 alignment, 4:1:0 has not
and its identical in the encoders
This is why it crashes, the patch corrects this difference.
I do not think thats a great differernce to have

If we keep the 4:2:0 and 4:1:0 difference in alignment then
what should i do about the encoder ?
adjust EDGE_WIDTH for 4:1:0 only ? because 4:2:0 has the buffer dimensions aligned correctly
so it doesnt need it bumped up?

That would result in 4:2:0 having the alignment from avcodec_align_dimensions2() for
decoder and encoder
and for 4:1:0 we would achieve something comparable with EDGE_WIDTH encoder side only
and hope the decoder doesnt need it

This just doesnt sound like a step in the right direction
Also this as a security fix should be simple

thx

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

It is a danger to trust the dream we wish for rather than
the science we have, -- Dr. Kenneth Brown

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

Re: [FFmpeg-devel] [PATCH 1/7] avcodec/utils: apply the same alignment to YUV410 as we do to YUV420 when motion estimation is used

[Michael Niedermayer]

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

On Mon, Jul 15, 2024 at 03:59:12PM +0200, Michael Niedermayer wrote:
> On Tue, Jun 18, 2024 at 07:28:18PM +0200, Andreas Rheinhardt wrote:
> > Michael Niedermayer:
> > > The snow encoder uses block based motion estimation which can read out of array if
> > > insufficient alignment is used
> > > 
> > > Fixes: out of array access
> > > Fixes: 68963/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-4979988435632128
> > > Fixes: 68969/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-6239933667803136.fuzz
> > > 
> > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavcodec/utils.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > > index 337c00e789a..7914f799041 100644
> > > --- a/libavcodec/utils.c
> > > +++ b/libavcodec/utils.c
> > > @@ -259,6 +259,9 @@ void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
> > >          if (s->codec_id == AV_CODEC_ID_SVQ1) {
> > >              w_align = 64;
> > >              h_align = 64;
> > > +        } else if (s->codec_id == AV_CODEC_ID_SNOW) {
> > > +            w_align = 16;
> > > +            h_align = 16;
> > >          }
> > >          break;
> > >      case AV_PIX_FMT_RGB555:
> > 
> > avcodec_align_dimensions2() is for decoders and happens to be used by
> > ff_encode_alloc_frame(), too. But decoders should not be required to add
> > more padding because the decoder needs more. Instead the encoder should
> > add more padding itself (by using more than 2 * EDGE_WIDTH).
> 
> If you have verified that the decoder alignment is sufficient and 4:1:0 does not
> need the alignment that 4:2:0 has for snow, then yes i can mess with some
> EDGE_WIDTH uses in snow to fix the encoder crash
> 
> Again the snow decoder for 4:2:0 has 16x16 alignment, 4:1:0 has not
> and its identical in the encoders
> This is why it crashes, the patch corrects this difference.
> I do not think thats a great differernce to have
> 
> If we keep the 4:2:0 and 4:1:0 difference in alignment then
> what should i do about the encoder ?
> adjust EDGE_WIDTH for 4:1:0 only ? because 4:2:0 has the buffer dimensions aligned correctly
> so it doesnt need it bumped up?
> 
> That would result in 4:2:0 having the alignment from avcodec_align_dimensions2() for
> decoder and encoder
> and for 4:1:0 we would achieve something comparable with EDGE_WIDTH encoder side only
> and hope the decoder doesnt need it
> 
> This just doesnt sound like a step in the right direction
> Also this as a security fix should be simple

Will apply the original patch with a note that its not the ideal solution
i dont want to leave this issue open while noone seems working on it

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle

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