[FFmpeg-devel] lavf/matroskaenc: Do not allow -reserve_index_space 1

Submitted by Carl Eugen Hoyos on Feb. 11, 2018, 7:42 p.m.

Details

Message ID CAB0OVGoui1jnZiju36WcmOFknUE46FMwwORYZat-42rOJamNAg@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Feb. 11, 2018, 7:42 p.m.
2018-02-11 20:23 GMT+01:00 Nicolas George <george@nsup.org>:
> Carl Eugen Hoyos (2018-02-11):
>> Attached patch fixes an assertion failure with the following command line:
>> $ ffmpeg -f lavfi -i testsrc -reserve_index_space 1 out.mkv

> Reading the code, it is pretty obvious that 0 is a valid value and
> forbidding it would be bad.

It is still the used default value...

Uglier alternative attached.

Carl Eugen

Comments

Nicolas George Feb. 11, 2018, 7:57 p.m.
Carl Eugen Hoyos (2018-02-11):
> 2018-02-11 20:23 GMT+01:00 Nicolas George <george@nsup.org>:
> > Carl Eugen Hoyos (2018-02-11):
> >> Attached patch fixes an assertion failure with the following command line:
> >> $ ffmpeg -f lavfi -i testsrc -reserve_index_space 1 out.mkv
> 
> > Reading the code, it is pretty obvious that 0 is a valid value and
> > forbidding it would be bad.
> 
> It is still the used default value...
> 
> Uglier alternative attached.
> 
> Carl Eugen

> From 6a08d7cb89294b81e87dac93bbf58627e5d37cec Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Sun, 11 Feb 2018 20:41:32 +0100
> Subject: [PATCH] lavf/matroskaenc: Force the minimum value for
>  -reserve_index_space to 2.
> 
> Fixes an assertion failure:
> Assertion size >= 2 failed at libavformat/matroskaenc.c:298
> ---
>  libavformat/matroskaenc.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f22c2ab..5950b4d 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2001,6 +2001,8 @@ static int mkv_write_header(AVFormatContext *s)
>      }
>      if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && mkv->reserve_cues_space) {
>          mkv->cues_pos = avio_tell(pb);

> +        if (mkv->reserve_cues_space == 1)
> +            mkv->reserve_cues_space++;
>          put_ebml_void(pb, mkv->reserve_cues_space);

I would have written it:

        put_ebml_void(pb, FFMAX(2, mkv->reserve_cues_space));

That is more or less equivalent to this patch.

But I do not maintain that file, and I do not know the actual meaning of
these fields any more than you.


>      }
>  

Regards,
Carl Eugen Hoyos Feb. 12, 2018, 9:51 p.m.
2018-02-11 20:57 GMT+01:00 Nicolas George <george@nsup.org>:
> Carl Eugen Hoyos (2018-02-11):
>> 2018-02-11 20:23 GMT+01:00 Nicolas George <george@nsup.org>:
>> > Carl Eugen Hoyos (2018-02-11):
>> >> Attached patch fixes an assertion failure with the following command line:
>> >> $ ffmpeg -f lavfi -i testsrc -reserve_index_space 1 out.mkv
>>
>> > Reading the code, it is pretty obvious that 0 is a valid value and
>> > forbidding it would be bad.
>>
>> It is still the used default value...
>>
>> Uglier alternative attached.
>>
>> Carl Eugen
>
>> From 6a08d7cb89294b81e87dac93bbf58627e5d37cec Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Sun, 11 Feb 2018 20:41:32 +0100
>> Subject: [PATCH] lavf/matroskaenc: Force the minimum value for
>>  -reserve_index_space to 2.
>>
>> Fixes an assertion failure:
>> Assertion size >= 2 failed at libavformat/matroskaenc.c:298
>> ---
>>  libavformat/matroskaenc.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index f22c2ab..5950b4d 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -2001,6 +2001,8 @@ static int mkv_write_header(AVFormatContext *s)
>>      }
>>      if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && mkv->reserve_cues_space) {
>>          mkv->cues_pos = avio_tell(pb);
>
>> +        if (mkv->reserve_cues_space == 1)
>> +            mkv->reserve_cues_space++;
>>          put_ebml_void(pb, mkv->reserve_cues_space);
>
> I would have written it:
>
>         put_ebml_void(pb, FFMAX(2, mkv->reserve_cues_space));
>
> That is more or less equivalent to this patch.

Applied my variant as I consider it (very slightly) more readable.

Thank you, Carl Eugen

Patch hide | download patch | download mbox

From 6a08d7cb89294b81e87dac93bbf58627e5d37cec Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Sun, 11 Feb 2018 20:41:32 +0100
Subject: [PATCH] lavf/matroskaenc: Force the minimum value for
 -reserve_index_space to 2.

Fixes an assertion failure:
Assertion size >= 2 failed at libavformat/matroskaenc.c:298
---
 libavformat/matroskaenc.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f22c2ab..5950b4d 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -2001,6 +2001,8 @@  static int mkv_write_header(AVFormatContext *s)
     }
     if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && mkv->reserve_cues_space) {
         mkv->cues_pos = avio_tell(pb);
+        if (mkv->reserve_cues_space == 1)
+            mkv->reserve_cues_space++;
         put_ebml_void(pb, mkv->reserve_cues_space);
     }
 
-- 
1.7.10.4