mbox series

[FFmpeg-devel,v3,0/3] checkasm: updated tests for sw_scale

Message ID 241b0204d8e34436b8fc785654ce48c8@amazon.com
Headers show
Series checkasm: updated tests for sw_scale | expand

Message

Swinney, Jonathan Aug. 13, 2022, 8:55 p.m. UTC
> We don't generally use stdbool in ffmpeg, even if it's C99 - just use a 
> plain int and 0/1.
Updated this.

> Other than that, the checkasm changes look fine (I coauthored part of 
> them - and your cleanup of my WIP patch looks good!).
Yes, thank you for the help on that!

> Hmm, can you elaborate on this bit? With only the first patch applied, the 
> checkasm test still succeeds.
That was leftover from when my test was broken. Is that that with the fixed
test, it works fine.

> Could we split this improvement for the existing codepath into a separate 
> preceding patch, to keep things a bit clearer?
I split it out. Let me know if I didn't split like you intended.

Thanks again for your review!

Jonathan Swinney

Comments

Martin Storsjö Aug. 16, 2022, 10:41 a.m. UTC | #1
On Sat, 13 Aug 2022, Swinney, Jonathan wrote:

>> We don't generally use stdbool in ffmpeg, even if it's C99 - just use a
>> plain int and 0/1.
> Updated this.
>
>> Other than that, the checkasm changes look fine (I coauthored part of
>> them - and your cleanup of my WIP patch looks good!).
> Yes, thank you for the help on that!
>
>> Hmm, can you elaborate on this bit? With only the first patch applied, the
>> checkasm test still succeeds.
> That was leftover from when my test was broken. Is that that with the fixed
> test, it works fine.
>
>> Could we split this improvement for the existing codepath into a separate
>> preceding patch, to keep things a bit clearer?
> I split it out. Let me know if I didn't split like you intended.

Thanks, this looked good to me, so I pushed them!

// Martin