[FFmpeg-devel] avcodec/cbs_av1: fix array size for ar_coeffs_cb_plus_128 and ar_coeffs_cr_plus_128

On Thu, Dec 12, 2019 at 12:15:14AM +0100, Michael Niedermayer wrote:
> On Wed, Dec 11, 2019 at 07:00:06PM -0300, James Almer wrote:
> > Taking into account the code
> > 
> > fb(2, ar_coeff_lag);
> > num_pos_luma = 2 * current->ar_coeff_lag * (current->ar_coeff_lag + 1);
> > if (current->num_y_points)
> >     num_pos_chroma = num_pos_luma + 1;
> > else
> >     num_pos_chroma = num_pos_luma;
> > 
> > Max value for ar_coeff_lag is 3 (two bits), for num_pos_luma 24, and for
> > num_pos_chroma 25.
> > 
> > Both ar_coeffs_cb_plus_128 and ar_coeffs_cr_plus_128 may have up to
> > num_pos_chroma values.
> > 
> > Signed-off-by: James Almer <jamrial@gmail.com>
> > ---
> >  libavcodec/cbs_av1.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> LGTM and i should have realized this when i looked at the fuzzer case 

This needs a update to fate though

TEST    wmav1-encode
Test cbs-av1-av1-1-b8-23-film_grain-50 failed. Look at tests/data/fate/cbs-av1-av1-1-b8-23-film_grain-50.err for details.
make: *** [fate-cbs-av1-av1-1-b8-23-film_grain-50] Error 1
Test cbs-av1-av1-1-b10-23-film_grain-50 failed. Look at tests/data/fate/cbs-av1-av1-1-b10-23-film_grain-50.err for details.
make: *** [fate-cbs-av1-av1-1-b10-23-film_grain-50] Error 1

Thanks

[...]

On 12/11/2019 9:33 PM, Michael Niedermayer wrote:
> On Thu, Dec 12, 2019 at 12:15:14AM +0100, Michael Niedermayer wrote:
>> On Wed, Dec 11, 2019 at 07:00:06PM -0300, James Almer wrote:
>>> Taking into account the code
>>>
>>> fb(2, ar_coeff_lag);
>>> num_pos_luma = 2 * current->ar_coeff_lag * (current->ar_coeff_lag + 1);
>>> if (current->num_y_points)
>>>     num_pos_chroma = num_pos_luma + 1;
>>> else
>>>     num_pos_chroma = num_pos_luma;
>>>
>>> Max value for ar_coeff_lag is 3 (two bits), for num_pos_luma 24, and for
>>> num_pos_chroma 25.
>>>
>>> Both ar_coeffs_cb_plus_128 and ar_coeffs_cr_plus_128 may have up to
>>> num_pos_chroma values.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>  libavcodec/cbs_av1.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> LGTM and i should have realized this when i looked at the fuzzer case 
> 
> This needs a update to fate though
> 
> --- ./tests/ref/fate/cbs-av1-av1-1-b8-23-film_grain-50	2019-12-11 15:53:24.616370434 +0100
> +++ tests/data/fate/cbs-av1-av1-1-b8-23-film_grain-50	2019-12-12 01:25:39.813093782 +0100
> @@ -1 +1 @@
> -93e578da4b2bdb70ce0a92262da930ec
> +2556d6d30469c647c6262130d8eb0531
> --- ./tests/ref/fate/cbs-av1-av1-1-b10-23-film_grain-50	2019-12-11 15:53:24.616370434 +0100
> +++ tests/data/fate/cbs-av1-av1-1-b10-23-film_grain-50	2019-12-12 01:25:39.825093782 +0100
> @@ -1 +1 @@
> -5fe6622ac6cb22dd10dfbe9e70e82d4e
> +aaddbf87bdf9b5ac4b1ab4fe802c410b
> TEST    wmav1-encode
> Test cbs-av1-av1-1-b8-23-film_grain-50 failed. Look at tests/data/fate/cbs-av1-av1-1-b8-23-film_grain-50.err for details.
> make: *** [fate-cbs-av1-av1-1-b8-23-film_grain-50] Error 1
> Test cbs-av1-av1-1-b10-23-film_grain-50 failed. Look at tests/data/fate/cbs-av1-av1-1-b10-23-film_grain-50.err for details.
> make: *** [fate-cbs-av1-av1-1-b10-23-film_grain-50] Error 1
> 
> Thanks

Great. So these tests are meant to ensure that the output is the same
with or without calling the corresponding metadata bsf, as it runs it in
passthrough mode. After this patch, it changed the output of both cases,
so it means the bsf is being used even if not explicitly called, and i'm
fairly sure it's the ivf muxer's fault.

I'll update it now, then will see about making another patch to stop
using the ivf muxer.