[FFmpeg-devel] [PATCH v2] avcodec/cbs_av1: add missing value constrains to point_y_value, point_cb_value and point_cr_value

[James Almer]

On 2/13/2020 8:08 PM, Mark Thompson wrote:
> On 13/02/2020 18:21, James Almer wrote:
>> On 2/13/2020 2:45 PM, Andreas Rheinhardt wrote:
>>> On Thu, Feb 13, 2020 at 4:08 PM James Almer <jamrial at gmail.com> wrote:
>>>
>>>> If i is greater than 0, it is a requirement of bitstream conformance that
>>>> point_y_value[ i ] is greater than point_y_value[ i - 1 ].
>>>> If i is greater than 0, it is a requirement of bitstream conformance that
>>>> point_cb_value[ i ] is greater than point_cb_value[ i - 1 ].
>>>> If i is greater than 0, it is a requirement of bitstream conformance that
>>>> point_cr_value[ i ] is greater than point_cr_value[ i - 1 ].
>>>>
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>> Better version. Now it can't overflow the min value, and will also
>>>> constrain
>>>> the max value to ensure v[i] > v[i-1] is always true.
>>>>
>>>
>>> How could the min value overflow at all? After all, the addition v[i - 1] +
>>> 1 is performed after promoting v[i - 1] to int. This is then losslessly
>>> converted to uint32_t. So your new version will not detect any more errors
>>> than the old version. It might error out earlier sometimes, but to do so it
>>> always computes the max.
>>> (With the old version it can happen that the min value is bigger than the
>>> max value which leads to the desired error (and an error message that might
>>> be confusing; avoiding this seems to be the only real advantage this new
>>> version has).)
>>>
>>> - Andreas
>>
>> I wasn't sure if the sum would be promoted or not, but in any case, the
>> core change is ensuring a given max value is constrained to always be
>> smaller than the next min allowed value.
> 
> Yeah.  With your change the error will be precise and diagnosed as early as possible (e.g. { 10, 30, 20, 40, 50 } -> ~"value[2] out of range: 20, but must be in [31,253]"), so it looks good to me.
> 
> Thanks,
> 
> - Mark

Applied, thanks.