diff mbox

[FFmpeg-devel] options_table: replace INT64_MAX with a sligthly smaller value

Message ID 2af51940-cd6f-26aa-585f-50acaf5d744c@googlemail.com
State Superseded
Headers show

Commit Message

Andreas Cadhalpun Nov. 19, 2016, 1:01 p.m. UTC 
On 19.11.2016 01:39, Michael Niedermayer wrote:
> On Fri, Nov 18, 2016 at 11:06:56PM +0100, Andreas Cadhalpun wrote:
>> AVOption.max is a double, which has not enough precision for INT64_MAX.
>>
>> It gets interpreted as INT64_MIN, when converted back to int64_t.
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
>> ---
>>  libavcodec/options_table.h | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
>> index 48de667..f117ce4 100644
>> --- a/libavcodec/options_table.h
>> +++ b/libavcodec/options_table.h
>> @@ -41,8 +41,10 @@
>>  
>>  #define AV_CODEC_DEFAULT_BITRATE 200*1000
>>  
>> +#define FF_INT64_MAX (INT64_MAX - 0x1000)
> 
> Missing documentation

Added a comment.

>> +
>>  static const AVOption avcodec_options[] = {
>> -{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, INT64_MAX, A|V|E},
>> +{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, FF_INT64_MAX, A|V|E},
>>  {"ab", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = 128*1000 }, 0, INT_MAX, A|E},
>>  {"bt", "Set video bitrate tolerance (in bits/s). In 1-pass mode, bitrate tolerance specifies how far "
>>         "ratecontrol is willing to deviate from the target average bitrate value. This is not related "
>> @@ -444,11 +446,11 @@ static const AVOption avcodec_options[] = {
>>  #if FF_API_PRIVATE_OPT
>>  {"min_prediction_order", NULL, OFFSET(min_prediction_order), AV_OPT_TYPE_INT, {.i64 = -1 }, INT_MIN, INT_MAX, A|E},
>>  {"max_prediction_order", NULL, OFFSET(max_prediction_order), AV_OPT_TYPE_INT, {.i64 = -1 }, INT_MIN, INT_MAX, A|E},
>> -{"timecode_frame_start", "GOP timecode frame start number, in non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, {.i64 = -1 }, -1, INT64_MAX, V|E},
>> +{"timecode_frame_start", "GOP timecode frame start number, in non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, {.i64 = -1 }, -1, FF_INT64_MAX, V|E},
>>  #endif
> 
>>  {"bits_per_raw_sample", NULL, OFFSET(bits_per_raw_sample), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX},
>> -{"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, INT64_MAX, A|E|D, "channel_layout"},
>> +{"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, FF_INT64_MAX, A|E|D, "channel_layout"},
>> -{"request_channel_layout", NULL, OFFSET(request_channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, INT64_MAX, A|D, "request_channel_layout"},
>> +{"request_channel_layout", NULL, OFFSET(request_channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, FF_INT64_MAX, A|D, "request_channel_layout"},
> 
> this doesnt feel right
> layout is not a scalar

Then why is it limited to positive values?
Currently setting either of those to "max" will actually cause them to be set to INT64_MIN.

Anyway, I dropped this part. New patch attached.

Best regards,
Andreas

Comments

Michael Niedermayer Nov. 20, 2016, 11:56 a.m. UTC | #1 
On Sat, Nov 19, 2016 at 02:01:54PM +0100, Andreas Cadhalpun wrote:
> On 19.11.2016 01:39, Michael Niedermayer wrote:
> > On Fri, Nov 18, 2016 at 11:06:56PM +0100, Andreas Cadhalpun wrote:
> >> AVOption.max is a double, which has not enough precision for INT64_MAX.
> >>
> >> It gets interpreted as INT64_MIN, when converted back to int64_t.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> >> ---
> >>  libavcodec/options_table.h | 10 ++++++----
> >>  1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> >> index 48de667..f117ce4 100644
> >> --- a/libavcodec/options_table.h
> >> +++ b/libavcodec/options_table.h
> >> @@ -41,8 +41,10 @@
> >>  
> >>  #define AV_CODEC_DEFAULT_BITRATE 200*1000
> >>  
> >> +#define FF_INT64_MAX (INT64_MAX - 0x1000)
> > 
> > Missing documentation
> 
> Added a comment.
> 

> >> +
> >>  static const AVOption avcodec_options[] = {
> >> -{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, INT64_MAX, A|V|E},
> >> +{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, FF_INT64_MAX, A|V|E},
> >>  {"ab", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = 128*1000 }, 0, INT_MAX, A|E},
> >>  {"bt", "Set video bitrate tolerance (in bits/s). In 1-pass mode, bitrate tolerance specifies how far "
> >>         "ratecontrol is willing to deviate from the target average bitrate value. This is not related "
> >> @@ -444,11 +446,11 @@ static const AVOption avcodec_options[] = {
> >>  #if FF_API_PRIVATE_OPT
> >>  {"min_prediction_order", NULL, OFFSET(min_prediction_order), AV_OPT_TYPE_INT, {.i64 = -1 }, INT_MIN, INT_MAX, A|E},
> >>  {"max_prediction_order", NULL, OFFSET(max_prediction_order), AV_OPT_TYPE_INT, {.i64 = -1 }, INT_MIN, INT_MAX, A|E},
> >> -{"timecode_frame_start", "GOP timecode frame start number, in non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, {.i64 = -1 }, -1, INT64_MAX, V|E},
> >> +{"timecode_frame_start", "GOP timecode frame start number, in non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, {.i64 = -1 }, -1, FF_INT64_MAX, V|E},
> >>  #endif
> > 
> >>  {"bits_per_raw_sample", NULL, OFFSET(bits_per_raw_sample), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX},
> >> -{"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, INT64_MAX, A|E|D, "channel_layout"},
> >> +{"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, FF_INT64_MAX, A|E|D, "channel_layout"},
> >> -{"request_channel_layout", NULL, OFFSET(request_channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, INT64_MAX, A|D, "request_channel_layout"},
> >> +{"request_channel_layout", NULL, OFFSET(request_channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, FF_INT64_MAX, A|D, "request_channel_layout"},
> > 
> > this doesnt feel right
> > layout is not a scalar
> 
> Then why is it limited to positive values?

it is a unsigend field, it cannot be negative
i posted a patch long ago that added support for UINT64_MAX,
ive updated it and taken care of the comments and will post that in a
moment. This also should take care of the other issues i hope

[...]
diff mbox

Patch

From 0f6fe6ae00d8176aa5a84f7ef1232d743d32e0dd Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
Date: Fri, 18 Nov 2016 23:00:59 +0100
Subject: [PATCH] options_table: replace INT64_MAX with a sligthly smaller
 value

AVOption.max is a double, which has not enough precision for INT64_MAX.

It gets interpreted as INT64_MIN, when converted back to int64_t.

Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
---
 libavcodec/options_table.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 48de667..377dade 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -41,8 +41,12 @@ 
 
 #define AV_CODEC_DEFAULT_BITRATE 200*1000
 
+// AVOption.max is a double, which has not enough precision for INT64_MAX.
+// It gets interpreted as INT64_MIN, when converted back to int64_t.
+#define FF_INT64_MAX (INT64_MAX - 0x1000)
+
 static const AVOption avcodec_options[] = {
-{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, INT64_MAX, A|V|E},
+{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, FF_INT64_MAX, A|V|E},
 {"ab", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT64, {.i64 = 128*1000 }, 0, INT_MAX, A|E},
 {"bt", "Set video bitrate tolerance (in bits/s). In 1-pass mode, bitrate tolerance specifies how far "
        "ratecontrol is willing to deviate from the target average bitrate value. This is not related "
@@ -444,7 +448,7 @@  static const AVOption avcodec_options[] = {
 #if FF_API_PRIVATE_OPT
 {"min_prediction_order", NULL, OFFSET(min_prediction_order), AV_OPT_TYPE_INT, {.i64 = -1 }, INT_MIN, INT_MAX, A|E},
 {"max_prediction_order", NULL, OFFSET(max_prediction_order), AV_OPT_TYPE_INT, {.i64 = -1 }, INT_MIN, INT_MAX, A|E},
-{"timecode_frame_start", "GOP timecode frame start number, in non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, {.i64 = -1 }, -1, INT64_MAX, V|E},
+{"timecode_frame_start", "GOP timecode frame start number, in non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, {.i64 = -1 }, -1, FF_INT64_MAX, V|E},
 #endif
 {"bits_per_raw_sample", NULL, OFFSET(bits_per_raw_sample), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX},
 {"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, INT64_MAX, A|E|D, "channel_layout"},
-- 
2.10.2