diff mbox series

[FFmpeg-devel,1/7,RFC] Revert "avcodec/adpcm_swf: support decoding multiple fixed-sized blocks at once"

Message ID 20201105231110.7772-1-michael@niedermayer.cc
State Accepted
Headers show
Series [FFmpeg-devel,1/7,RFC] Revert "avcodec/adpcm_swf: support decoding multiple fixed-sized blocks at once" | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Michael Niedermayer Nov. 5, 2020, 11:11 p.m. UTC
The reverted code split at block_align boundaries, but there was already code
which splits at a hardcoded 4096 sample boundary.

reverting this seemed like the easiest fix but this is a RFC in case another solution
is preferred

Fixes: out of array write
Fixes: 26821/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ADPCM_SWF_fuzzer-5764465137811456

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
This reverts commit e9dd73d30d09043446ac6dd7b8ad31e557873852.
---
 libavcodec/adpcm.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

Comments

Zane van Iperen Nov. 6, 2020, 1:51 a.m. UTC | #1
On 6/11/20 9:11 am, Michael Niedermayer wrote:
> The reverted code split at block_align boundaries, but there was already code
> which splits at a hardcoded 4096 sample boundary.
> 
> reverting this seemed like the easiest fix but this is a RFC in case another solution
> is preferred
> 

This breaks playback when adpcm_swf is used in WAVs. block_align needs to be respected
otherwise everything is garbled.
When coming from a SWF file, block_align isn't set so it reverts to the legacy behaviour.

See https://trac.ffmpeg.org/ticket/5829
Zane van Iperen Nov. 6, 2020, 12:37 p.m. UTC | #2
On 6/11/20 11:51 am, Zane van Iperen wrote:
> 
> 
> On 6/11/20 9:11 am, Michael Niedermayer wrote:
>> The reverted code split at block_align boundaries, but there was already code
>> which splits at a hardcoded 4096 sample boundary.
>>
>> reverting this seemed like the easiest fix but this is a RFC in case another solution
>> is preferred
>>
> 
> This breaks playback when adpcm_swf is used in WAVs. block_align needs to be respected
> otherwise everything is garbled.
> When coming from a SWF file, block_align isn't set so it reverts to the legacy behaviour.
> 
> See https://trac.ffmpeg.org/ticket/5829
> 
> 

Never mind, this behaviour is wrong. Turns out the encoder was producing invalid frames and this
simply covered-up the issue. Revert away!

I'll send out a fix for the encoder soon.
diff mbox series

Patch

diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
index 701b125c47..d018c1f91b 100644
--- a/libavcodec/adpcm.c
+++ b/libavcodec/adpcm.c
@@ -880,7 +880,7 @@  static int get_nb_samples(AVCodecContext *avctx, GetByteContext *gb,
     }
     case AV_CODEC_ID_ADPCM_SWF:
     {
-        int buf_bits       = (avctx->block_align ? avctx->block_align : buf_size) * 8 - 2;
+        int buf_bits       = buf_size * 8 - 2;
         int nbits          = (bytestream2_get_byte(gb) >> 6) + 2;
         int block_hdr_size = 22 * ch;
         int block_size     = block_hdr_size + nbits * ch * 4095;
@@ -889,9 +889,6 @@  static int get_nb_samples(AVCodecContext *avctx, GetByteContext *gb,
         nb_samples         = nblocks * 4096;
         if (bits_left >= block_hdr_size)
             nb_samples += 1 + (bits_left - block_hdr_size) / (nbits * ch);
-
-        if (avctx->block_align)
-            nb_samples *= buf_size / avctx->block_align;
         break;
     }
     case AV_CODEC_ID_ADPCM_THP:
@@ -1770,17 +1767,9 @@  static int adpcm_decode_frame(AVCodecContext *avctx, void *data,
         }
         break;
     case AV_CODEC_ID_ADPCM_SWF:
-    {
-        const int nb_blocks  = avctx->block_align ? avpkt->size / avctx->block_align : 1;
-        const int block_size = avctx->block_align ? avctx->block_align : avpkt->size;
-
-        for (int block = 0; block < nb_blocks; block++) {
-            adpcm_swf_decode(avctx, buf + block * block_size, block_size, samples);
-            samples += nb_samples / nb_blocks;
-        }
+        adpcm_swf_decode(avctx, buf, buf_size, samples);
         bytestream2_seek(&gb, 0, SEEK_END);
         break;
-    }
     case AV_CODEC_ID_ADPCM_YAMAHA:
         for (n = nb_samples >> (1 - st); n > 0; n--) {
             int v = bytestream2_get_byteu(&gb);