Re: [FFmpeg-devel] [PATCH v3] avformat/nutdec: Add check for avformat_new_stream

Re: [FFmpeg-devel] [PATCH v3] avformat/nutdec: Add check for avformat_new_stream

[Jiasheng Jiang]

Michael Niedermayer:
>> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
>> index 0a8a700acf..32a4f6bbcb 100644
>> --- a/libavformat/nutdec.c
>> +++ b/libavformat/nutdec.c
>> @@ -351,8 +351,13 @@ static int decode_main_header(NUTContext *nut)
>>          ret = AVERROR(ENOMEM);
>>          goto fail;
>>      }
>> -    for (i = 0; i < stream_count; i++)
>> -        avformat_new_stream(s, NULL);
>> +    for (i = 0; i < stream_count; i++) {
>> +        if (!avformat_new_stream(s, NULL)) {
>> +            av_freep(nut->stream);
> 
> is this actually needed or isnt this freed anyway ?
> also if needed thats the wrong pointer 
> av_freep(&nut->stream)

OK, I will remove it and submit v4.

Jiang

_______________________________________________
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 v3] avformat/nutdec: Add check for avformat_new_stream

[Michael Niedermayer]

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

On Tue, Feb 22, 2022 at 10:41:51AM +0800, Jiasheng Jiang wrote:
> As the potential failure of the memory allocation,
> the avformat_new_stream() could return NULL pointer.
> Therefore, it should be better to check it and return
> error if fails.
> Also, the caller, nut_read_header(), needs to deal with
> the return value of the decode_main_header() and return
> error if memory allocation fails.
> And 'time_base_count' has already checked and it
> will return AVERROR_INVALIDDATA if fails, which is different
> from ENOMEM.
> 
> Fixes: 619d8e2e58 ("updating nut demuxer to latest spec no muxing yet no index yet no seeking yet libnuts crcs dont match mine (didnt investigate yet) samplerate is stored wrong by libnut (demuxer has a workaround) code is not clean or beautifull yet, but i thought its better to commit early before someone unneccesarily wastes his time duplicating the work demuxer split from muxer")
> 
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
> Changelog:
> 
> v1 -> v2
> 
> * Change 1. Add the error handling for ENOMEM from decode_main_header()
> in nut_read_header().
> * Change 2. Check for the 'time_base_count'.
> 
> v2 -> v3
> 
> * Change 1. Remove the check for 'time_base_count'.
> * Change 2. Change the av_free to av_freep.
> ---
>  libavformat/nutdec.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> index 0a8a700acf..32a4f6bbcb 100644
> --- a/libavformat/nutdec.c
> +++ b/libavformat/nutdec.c
> @@ -351,8 +351,13 @@ static int decode_main_header(NUTContext *nut)
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
> -    for (i = 0; i < stream_count; i++)
> -        avformat_new_stream(s, NULL);
> +    for (i = 0; i < stream_count; i++) {
> +        if (!avformat_new_stream(s, NULL)) {
> +            av_freep(nut->stream);

is this actually needed or isnt this freed anyway ?
also if needed thats the wrong pointer 
av_freep(&nut->stream)

[...]

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA

[-- 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 v3] avformat/nutdec: Add check for avformat_new_stream

[Jiasheng Jiang]

As the potential failure of the memory allocation,
the avformat_new_stream() could return NULL pointer.
Therefore, it should be better to check it and return
error if fails.
Also, the caller, nut_read_header(), needs to deal with
the return value of the decode_main_header() and return
error if memory allocation fails.
And 'time_base_count' has already checked and it
will return AVERROR_INVALIDDATA if fails, which is different
from ENOMEM.

Fixes: 619d8e2e58 ("updating nut demuxer to latest spec no muxing yet no index yet no seeking yet libnuts crcs dont match mine (didnt investigate yet) samplerate is stored wrong by libnut (demuxer has a workaround) code is not clean or beautifull yet, but i thought its better to commit early before someone unneccesarily wastes his time duplicating the work demuxer split from muxer")

Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
Changelog:

v1 -> v2

* Change 1. Add the error handling for ENOMEM from decode_main_header()
in nut_read_header().
* Change 2. Check for the 'time_base_count'.

v2 -> v3

* Change 1. Remove the check for 'time_base_count'.
* Change 2. Change the av_free to av_freep.
---
 libavformat/nutdec.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index 0a8a700acf..32a4f6bbcb 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -351,8 +351,13 @@ static int decode_main_header(NUTContext *nut)
         ret = AVERROR(ENOMEM);
         goto fail;
     }
-    for (i = 0; i < stream_count; i++)
-        avformat_new_stream(s, NULL);
+    for (i = 0; i < stream_count; i++) {
+        if (!avformat_new_stream(s, NULL)) {
+            av_freep(nut->stream);
+            ret = AVERROR(ENOMEM);
+            goto fail;
+        }
+    }
 
     return 0;
 fail:
@@ -800,19 +805,23 @@ static int nut_read_header(AVFormatContext *s)
     NUTContext *nut = s->priv_data;
     AVIOContext *bc = s->pb;
     int64_t pos;
-    int initialized_stream_count;
+    int initialized_stream_count, ret;
 
     nut->avf = s;
 
     /* main header */
     pos = 0;
+    ret = 0;
     do {
+        if (ret == AVERROR(ENOMEM))
+            return ret;
+
         pos = find_startcode(bc, MAIN_STARTCODE, pos) + 1;
         if (pos < 0 + 1) {
             av_log(s, AV_LOG_ERROR, "No main startcode found.\n");
             return AVERROR_INVALIDDATA;
         }
-    } while (decode_main_header(nut) < 0);
+    } while ((ret = decode_main_header(nut)) < 0);
 
     /* stream headers */
     pos = 0;
-- 
2.25.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".