[FFmpeg-devel] [PATCH 1/2] avcodec/speexdec: relax the extradata check for the speex string

[FFmpeg-devel] [PATCH 1/2] avcodec/speexdec: relax the extradata check for the speex string

[James Almer]

There could be bogus bytes at the start, as is the case of
vp5/potter512-400-partial.avi from the FATE suite, which could be a case of bad
remuxing from an OGG source.

Partially fixes decoding of vp5/potter512-400-partial.avi

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/speexdec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/speexdec.c b/libavcodec/speexdec.c
index 08c7e77e7d..c73b2a7ec2 100644
--- a/libavcodec/speexdec.c
+++ b/libavcodec/speexdec.c
@@ -52,6 +52,7 @@
  */
 
 #include "libavutil/avassert.h"
+#include "libavutil/avstring.h"
 #include "libavutil/float_dsp.h"
 #include "avcodec.h"
 #include "bytestream.h"
@@ -1397,9 +1398,9 @@ static int parse_speex_extradata(AVCodecContext *avctx,
     const uint8_t *extradata, int extradata_size)
 {
     SpeexContext *s = avctx->priv_data;
-    const uint8_t *buf = extradata;
+    const uint8_t *buf = av_strnstr(extradata, "Speex   ", extradata_size);
 
-    if (memcmp(buf, "Speex   ", 8))
+    if (!buf)
         return AVERROR_INVALIDDATA;
 
     buf += 28;
-- 
2.43.0

_______________________________________________
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/2] avcodec/speexdec: fix setting frame_size from extradata

[James Almer]

Finishes fixing vp5/potter512-400-partial.avi

The fate-matroska-ms-mode test ref is updated to reflect that the Speex decoder
can now read the stream.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/speexdec.c           | 4 +---
 tests/ref/fate/matroska-ms-mode | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/libavcodec/speexdec.c b/libavcodec/speexdec.c
index c73b2a7ec2..51c5834769 100644
--- a/libavcodec/speexdec.c
+++ b/libavcodec/speexdec.c
@@ -1420,9 +1420,7 @@ static int parse_speex_extradata(AVCodecContext *avctx,
     if (s->nb_channels <= 0 || s->nb_channels > 2)
         return AVERROR_INVALIDDATA;
     s->bitrate = bytestream_get_le32(&buf);
-    s->frame_size = bytestream_get_le32(&buf);
-    if (s->frame_size < NB_FRAME_SIZE << s->mode)
-        return AVERROR_INVALIDDATA;
+    s->frame_size = (1 + (s->mode > 0)) * bytestream_get_le32(&buf);
     s->vbr = bytestream_get_le32(&buf);
     s->frames_per_packet = bytestream_get_le32(&buf);
     if (s->frames_per_packet <= 0 ||
diff --git a/tests/ref/fate/matroska-ms-mode b/tests/ref/fate/matroska-ms-mode
index 5c91209910..0e31c990dc 100644
--- a/tests/ref/fate/matroska-ms-mode
+++ b/tests/ref/fate/matroska-ms-mode
@@ -1,4 +1,4 @@
-a2897e3951b0054d0fa31fe51860444f *tests/data/fate/matroska-ms-mode.matroska
+e7f44cd6a5c0f45fea11874afb8c1c0d *tests/data/fate/matroska-ms-mode.matroska
 413103 tests/data/fate/matroska-ms-mode.matroska
 #extradata 0:       40, 0x54290c93
 #extradata 1:      114, 0xb6c80771
-- 
2.43.0

_______________________________________________
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/2] avcodec/speexdec: fix setting frame_size from extradata

[Andreas Rheinhardt]

James Almer:
> Finishes fixing vp5/potter512-400-partial.avi
> 
> The fate-matroska-ms-mode test ref is updated to reflect that the Speex decoder
> can now read the stream.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/speexdec.c           | 4 +---
>  tests/ref/fate/matroska-ms-mode | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/speexdec.c b/libavcodec/speexdec.c
> index c73b2a7ec2..51c5834769 100644
> --- a/libavcodec/speexdec.c
> +++ b/libavcodec/speexdec.c
> @@ -1420,9 +1420,7 @@ static int parse_speex_extradata(AVCodecContext *avctx,
>      if (s->nb_channels <= 0 || s->nb_channels > 2)
>          return AVERROR_INVALIDDATA;
>      s->bitrate = bytestream_get_le32(&buf);
> -    s->frame_size = bytestream_get_le32(&buf);
> -    if (s->frame_size < NB_FRAME_SIZE << s->mode)
> -        return AVERROR_INVALIDDATA;
> +    s->frame_size = (1 + (s->mode > 0)) * bytestream_get_le32(&buf);
>      s->vbr = bytestream_get_le32(&buf);
>      s->frames_per_packet = bytestream_get_le32(&buf);
>      if (s->frames_per_packet <= 0 ||
> diff --git a/tests/ref/fate/matroska-ms-mode b/tests/ref/fate/matroska-ms-mode
> index 5c91209910..0e31c990dc 100644
> --- a/tests/ref/fate/matroska-ms-mode
> +++ b/tests/ref/fate/matroska-ms-mode
> @@ -1,4 +1,4 @@
> -a2897e3951b0054d0fa31fe51860444f *tests/data/fate/matroska-ms-mode.matroska
> +e7f44cd6a5c0f45fea11874afb8c1c0d *tests/data/fate/matroska-ms-mode.matroska
>  413103 tests/data/fate/matroska-ms-mode.matroska
>  #extradata 0:       40, 0x54290c93
>  #extradata 1:      114, 0xb6c80771

This changes the checksum of the generated Matroska file; presumably
some header parameter is now set differently without affecting the size
of the generated file (which one?). But this means that this test
probably needs an explicit dependency on the speex decoder (which it
currently doesn't have).

- 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 2/2] avcodec/speexdec: fix setting frame_size from extradata

[James Almer]

On 1/24/2024 11:35 AM, Andreas Rheinhardt wrote:
> James Almer:
>> Finishes fixing vp5/potter512-400-partial.avi
>>
>> The fate-matroska-ms-mode test ref is updated to reflect that the Speex decoder
>> can now read the stream.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/speexdec.c           | 4 +---
>>   tests/ref/fate/matroska-ms-mode | 2 +-
>>   2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/speexdec.c b/libavcodec/speexdec.c
>> index c73b2a7ec2..51c5834769 100644
>> --- a/libavcodec/speexdec.c
>> +++ b/libavcodec/speexdec.c
>> @@ -1420,9 +1420,7 @@ static int parse_speex_extradata(AVCodecContext *avctx,
>>       if (s->nb_channels <= 0 || s->nb_channels > 2)
>>           return AVERROR_INVALIDDATA;
>>       s->bitrate = bytestream_get_le32(&buf);
>> -    s->frame_size = bytestream_get_le32(&buf);
>> -    if (s->frame_size < NB_FRAME_SIZE << s->mode)
>> -        return AVERROR_INVALIDDATA;
>> +    s->frame_size = (1 + (s->mode > 0)) * bytestream_get_le32(&buf);
>>       s->vbr = bytestream_get_le32(&buf);
>>       s->frames_per_packet = bytestream_get_le32(&buf);
>>       if (s->frames_per_packet <= 0 ||
>> diff --git a/tests/ref/fate/matroska-ms-mode b/tests/ref/fate/matroska-ms-mode
>> index 5c91209910..0e31c990dc 100644
>> --- a/tests/ref/fate/matroska-ms-mode
>> +++ b/tests/ref/fate/matroska-ms-mode
>> @@ -1,4 +1,4 @@
>> -a2897e3951b0054d0fa31fe51860444f *tests/data/fate/matroska-ms-mode.matroska
>> +e7f44cd6a5c0f45fea11874afb8c1c0d *tests/data/fate/matroska-ms-mode.matroska
>>   413103 tests/data/fate/matroska-ms-mode.matroska
>>   #extradata 0:       40, 0x54290c93
>>   #extradata 1:      114, 0xb6c80771
> 
> This changes the checksum of the generated Matroska file; presumably
> some header parameter is now set differently without affecting the size
> of the generated file (which one?). But this means that this test

mkvinfo for the output, pre and post patch:

--- old.txt     2024-01-24 15:48:57.639516100 -0300
+++ new.txt     2024-01-24 15:49:01.301839500 -0300
@@ -42,7 +42,7 @@
  |  + Audio track
  |   + Channels: 1
  |   + Sampling frequency: 32000
-|   + Bit depth: 16
+|   + Bit depth: 32
  |  + Maximum block additional ID: 0
  |  + Codec's private data: size 132 (format tag: 0xa109)
  |+ Tags

Post patch the audio stream is reported as FLT AVSampleFormat.

> probably needs an explicit dependency on the speex decoder (which it
> currently doesn't have).

Can add one.

> 
> - 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".
_______________________________________________
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/2] avcodec/speexdec: fix setting frame_size from extradata

[Michael Niedermayer]

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

On Fri, Jan 19, 2024 at 10:47:30PM -0300, James Almer wrote:
> Finishes fixing vp5/potter512-400-partial.avi
> 
> The fate-matroska-ms-mode test ref is updated to reflect that the Speex decoder
> can now read the stream.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/speexdec.c           | 4 +---
>  tests/ref/fate/matroska-ms-mode | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)

Theres plenty of code remaining after this change that assumes NB_FRAME_SIZE
are available and writes that also.

There was a testcase reporrted that causes this and many others
ill forward the testcase.

==18747== Invalid write of size 4
==18747==    at 0xD1CFC3: sb_decode (speexdec.c:1260)
==18747==    by 0xD1E5CE: speex_decode_frame (speexdec.c:1557)
==18747==    by 0x998846: decode_simple_internal (decode.c:430)
==18747==    by 0x998DD7: decode_simple_receive_frame (decode.c:609)
==18747==    by 0x998F47: decode_receive_frame_internal (decode.c:637)
==18747==    by 0x99930C: avcodec_send_packet (decode.c:734)
==18747==    by 0x669D2F: try_decode_frame (demux.c:2126)
==18747==    by 0x66CAA0: avformat_find_stream_info (demux.c:2809)
==18747==    by 0x24E9C2: ifile_open (ffmpeg_demux.c:1663)
==18747==    by 0x2755BE: open_files (ffmpeg_opt.c:1333)
==18747==    by 0x275780: ffmpeg_parse_options (ffmpeg_opt.c:1373)
==18747==    by 0x289702: main (ffmpeg.c:1032)
==18747==  Address 0x16a170c0 is 0 bytes after a block of size 1,536 alloc'd
==18747==    at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18747==    by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18747==    by 0x13E3FE6: av_malloc (mem.c:105)
==18747==    by 0x13BFBFD: av_buffer_alloc (buffer.c:82)
==18747==    by 0x13C0645: pool_alloc_buffer (buffer.c:362)
==18747==    by 0x13C07C6: av_buffer_pool_get (buffer.c:401)
==18747==    by 0xA46052: audio_get_buffer (get_buffer.c:203)
==18747==    by 0xA4648D: avcodec_default_get_buffer2 (get_buffer.c:278)
==18747==    by 0x99B967: ff_get_buffer (decode.c:1673)
==18747==    by 0xD1E52E: speex_decode_frame (speexdec.c:1552)
==18747==    by 0x998846: decode_simple_internal (decode.c:430)
==18747==    by 0x998DD7: decode_simple_receive_frame (decode.c:609)
==18747==    by 0x998F47: decode_receive_frame_internal (decode.c:637)
==18747==    by 0x99930C: avcodec_send_packet (decode.c:734)
==18747==    by 0x669D2F: try_decode_frame (demux.c:2126)
==18747==    by 0x66CAA0: avformat_find_stream_info (demux.c:2809)
==18747==    by 0x24E9C2: ifile_open (ffmpeg_demux.c:1663)
==18747==    by 0x2755BE: open_files (ffmpeg_opt.c:1333)
==18747==    by 0x275780: ffmpeg_parse_options (ffmpeg_opt.c:1373)
==18747==    by 0x289702: main (ffmpeg.c:1032)

thx

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato

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