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

[Mark Thompson]

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