subject:"\"\\\\\\\[FFmpeg\\\\\\\-devel\\\\\\\] \\\\\\\[PATCH\\\\\\\] avcodec\\\\\\\/cbs_av1\\\\\\\: fix range of values for Mastering Display Color Volume Metadata OBUs\""

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

2019-03-25 Thread James Almer

On 3/23/2019 2:30 PM, James Almer wrote:
> Signed-off-by: James Almer 
> ---
>> Does something like
>> FFMIN(((uint64_t)current->luminance_max << 6) - 1, MAX_UINT_BITS(32))
>> do the right thing?
> 
> Yes. Fixed and a comment added.
> 
>  libavcodec/cbs_av1_syntax_template.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1_syntax_template.c 
> b/libavcodec/cbs_av1_syntax_template.c
> index 48f4fab514..76eb90b279 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -1637,15 +1637,18 @@ static int 
> FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw,
>  int err, i;
>  
>  for (i = 0; i < 3; i++) {
> -fcs(16, primary_chromaticity_x[i], 0, 5, 1, i);
> -fcs(16, primary_chromaticity_y[i], 0, 5, 1, i);
> +fbs(16, primary_chromaticity_x[i], 1, i);
> +fbs(16, primary_chromaticity_y[i], 1, i);
>  }
>  
> -fc(16, white_point_chromaticity_x, 0, 5);
> -fc(16, white_point_chromaticity_y, 0, 5);
> +fb(16, white_point_chromaticity_x);
> +fb(16, white_point_chromaticity_y);
>  
>  fc(32, luminance_max, 1, MAX_UINT_BITS(32));
> -fc(32, luminance_min, 0, current->luminance_max >> 6);
> +// luminance_min must be lower than luminance_max. Convert luminance_max 
> from
> +// 24.8 fixed point to 18.14 fixed point in order to compare them.
> +fc(32, luminance_min, 0, FFMIN(((uint64_t)current->luminance_max << 6) - 
> 1,
> +   MAX_UINT_BITS(32)));
>  
>  return 0;
>  }

Pushed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

2019-03-23 Thread James Almer

Signed-off-by: James Almer 
---
> Does something like
> FFMIN(((uint64_t)current->luminance_max << 6) - 1, MAX_UINT_BITS(32))
> do the right thing?

Yes. Fixed and a comment added.

 libavcodec/cbs_av1_syntax_template.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 48f4fab514..76eb90b279 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1637,15 +1637,18 @@ static int 
FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw,
 int err, i;
 
 for (i = 0; i < 3; i++) {
-fcs(16, primary_chromaticity_x[i], 0, 5, 1, i);
-fcs(16, primary_chromaticity_y[i], 0, 5, 1, i);
+fbs(16, primary_chromaticity_x[i], 1, i);
+fbs(16, primary_chromaticity_y[i], 1, i);
 }
 
-fc(16, white_point_chromaticity_x, 0, 5);
-fc(16, white_point_chromaticity_y, 0, 5);
+fb(16, white_point_chromaticity_x);
+fb(16, white_point_chromaticity_y);
 
 fc(32, luminance_max, 1, MAX_UINT_BITS(32));
-fc(32, luminance_min, 0, current->luminance_max >> 6);
+// luminance_min must be lower than luminance_max. Convert luminance_max 
from
+// 24.8 fixed point to 18.14 fixed point in order to compare them.
+fc(32, luminance_min, 0, FFMIN(((uint64_t)current->luminance_max << 6) - 1,
+   MAX_UINT_BITS(32)));
 
 return 0;
 }
-- 
2.21.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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

2019-03-23 Thread Mark Thompson

On 23/03/2019 14:48, James Almer wrote:
> On 3/23/2019 9:46 AM, Mark Thompson wrote:
>> On 21/03/2019 18:37, James Almer wrote:
>>> Signed-off-by: James Almer 
>>> ---
>>>  libavcodec/cbs_av1_syntax_template.c | 10 +-
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavcodec/cbs_av1_syntax_template.c 
>>> b/libavcodec/cbs_av1_syntax_template.c
>>> index 48f4fab514..93bba1e5ad 100644
>>> --- a/libavcodec/cbs_av1_syntax_template.c
>>> +++ b/libavcodec/cbs_av1_syntax_template.c
>>> @@ -1637,15 +1637,15 @@ static int 
>>> FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw,
>>>  int err, i;
>>>  
>>>  for (i = 0; i < 3; i++) {
>>> -fcs(16, primary_chromaticity_x[i], 0, 5, 1, i);
>>> -fcs(16, primary_chromaticity_y[i], 0, 5, 1, i);
>>> +fbs(16, primary_chromaticity_x[i], 1, i);
>>> +fbs(16, primary_chromaticity_y[i], 1, i);
>>>  }
>>>  
>>> -fc(16, white_point_chromaticity_x, 0, 5);
>>> -fc(16, white_point_chromaticity_y, 0, 5);
>>> +fb(16, white_point_chromaticity_x);
>>> +fb(16, white_point_chromaticity_y);
>>>  
>>>  fc(32, luminance_max, 1, MAX_UINT_BITS(32));
>>> -fc(32, luminance_min, 0, current->luminance_max >> 6);
>>> +fc(32, luminance_min, 0, (current->luminance_max << 6) - 1);
>>
>> << 6 in uint32_t can overflow.
> 
> What do you suggest? Casting to uint64_t will get the value clipped when
> passed to ff_cbs_read_unsigned() and potentially result in a failed
> range check.

Does something like
FFMIN(((uint64_t)current->luminance_max << 6) - 1, MAX_UINT_BITS(32))
do the right thing?

>>
>> Given the confusion in the previous value, maybe a comment to explain where 
>> the upper bound for min is coming from too?
>>
>>>  
>>>  return 0;
>>>  }
>>>
>> LGTM with that.
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

2019-03-23 Thread James Almer

On 3/23/2019 9:46 AM, Mark Thompson wrote:
> On 21/03/2019 18:37, James Almer wrote:
>> Signed-off-by: James Almer 
>> ---
>>  libavcodec/cbs_av1_syntax_template.c | 10 +-
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/cbs_av1_syntax_template.c 
>> b/libavcodec/cbs_av1_syntax_template.c
>> index 48f4fab514..93bba1e5ad 100644
>> --- a/libavcodec/cbs_av1_syntax_template.c
>> +++ b/libavcodec/cbs_av1_syntax_template.c
>> @@ -1637,15 +1637,15 @@ static int 
>> FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw,
>>  int err, i;
>>  
>>  for (i = 0; i < 3; i++) {
>> -fcs(16, primary_chromaticity_x[i], 0, 5, 1, i);
>> -fcs(16, primary_chromaticity_y[i], 0, 5, 1, i);
>> +fbs(16, primary_chromaticity_x[i], 1, i);
>> +fbs(16, primary_chromaticity_y[i], 1, i);
>>  }
>>  
>> -fc(16, white_point_chromaticity_x, 0, 5);
>> -fc(16, white_point_chromaticity_y, 0, 5);
>> +fb(16, white_point_chromaticity_x);
>> +fb(16, white_point_chromaticity_y);
>>  
>>  fc(32, luminance_max, 1, MAX_UINT_BITS(32));
>> -fc(32, luminance_min, 0, current->luminance_max >> 6);
>> +fc(32, luminance_min, 0, (current->luminance_max << 6) - 1);
> 
> << 6 in uint32_t can overflow.

What do you suggest? Casting to uint64_t will get the value clipped when
passed to ff_cbs_read_unsigned() and potentially result in a failed
range check.

> 
> Given the confusion in the previous value, maybe a comment to explain where 
> the upper bound for min is coming from too?
> 
>>  
>>  return 0;
>>  }
>>
> LGTM with that.
> 
> Thanks,
> 
> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

2019-03-23 Thread Mark Thompson

On 21/03/2019 18:37, James Almer wrote:
> Signed-off-by: James Almer 
> ---
>  libavcodec/cbs_av1_syntax_template.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1_syntax_template.c 
> b/libavcodec/cbs_av1_syntax_template.c
> index 48f4fab514..93bba1e5ad 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -1637,15 +1637,15 @@ static int 
> FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw,
>  int err, i;
>  
>  for (i = 0; i < 3; i++) {
> -fcs(16, primary_chromaticity_x[i], 0, 5, 1, i);
> -fcs(16, primary_chromaticity_y[i], 0, 5, 1, i);
> +fbs(16, primary_chromaticity_x[i], 1, i);
> +fbs(16, primary_chromaticity_y[i], 1, i);
>  }
>  
> -fc(16, white_point_chromaticity_x, 0, 5);
> -fc(16, white_point_chromaticity_y, 0, 5);
> +fb(16, white_point_chromaticity_x);
> +fb(16, white_point_chromaticity_y);
>  
>  fc(32, luminance_max, 1, MAX_UINT_BITS(32));
> -fc(32, luminance_min, 0, current->luminance_max >> 6);
> +fc(32, luminance_min, 0, (current->luminance_max << 6) - 1);

<< 6 in uint32_t can overflow.

Given the confusion in the previous value, maybe a comment to explain where the 
upper bound for min is coming from too?

>  
>  return 0;
>  }
> 
LGTM with that.

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

2019-03-21 Thread James Almer

Signed-off-by: James Almer 
---
 libavcodec/cbs_av1_syntax_template.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 48f4fab514..93bba1e5ad 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1637,15 +1637,15 @@ static int 
FUNC(metadata_hdr_mdcv)(CodedBitstreamContext *ctx, RWContext *rw,
 int err, i;
 
 for (i = 0; i < 3; i++) {
-fcs(16, primary_chromaticity_x[i], 0, 5, 1, i);
-fcs(16, primary_chromaticity_y[i], 0, 5, 1, i);
+fbs(16, primary_chromaticity_x[i], 1, i);
+fbs(16, primary_chromaticity_y[i], 1, i);
 }
 
-fc(16, white_point_chromaticity_x, 0, 5);
-fc(16, white_point_chromaticity_y, 0, 5);
+fb(16, white_point_chromaticity_x);
+fb(16, white_point_chromaticity_y);
 
 fc(32, luminance_max, 1, MAX_UINT_BITS(32));
-fc(32, luminance_min, 0, current->luminance_max >> 6);
+fc(32, luminance_min, 0, (current->luminance_max << 6) - 1);
 
 return 0;
 }
-- 
2.21.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

[FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

[FFmpeg-devel] [PATCH] avcodec/cbs_av1: fix range of values for Mastering Display Color Volume Metadata OBUs

6 matches

Site Navigation

Mail list logo

Footer information