Re: [FFmpeg-devel] [PATCH v4 01/16] pixfmt: add AV_PIX_FMT_GBRAP32

From: Marton Balint <cus@passwd.hu>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v4 01/16] pixfmt: add AV_PIX_FMT_GBRAP32
Date: Fri, 14 Mar 2025 19:34:11 +0100 (CET)
Message-ID: <f38e96be-8504-74a9-4534-8fc049a6b05d@passwd.hu> (raw)
In-Reply-To: <ac7412f8-76ab-4e6c-ac08-daaed8c31ade@lynne.ee>

On Fri, 14 Mar 2025, Lynne wrote:

> On 14/03/2025 16:13, Michael Niedermayer wrote:
>>  On Thu, Mar 13, 2025 at 06:03:33PM +0100, Lynne wrote:
>>>  This commit adds a 32-bit *integer* planar RGBA format.
>>>  Vulkan FFv1 decoding is best performed on separate planes, rather than
>>>  packed RGBA (i.e. RGBA128), hence this is useful as an intermediate
>>>  format.
>>>  ---
>>>    libavutil/pixdesc.c | 28 ++++++++++++++++++++++++++++
>>>    libavutil/pixfmt.h  |  4 ++++
>>>    2 files changed, 32 insertions(+)
>>
>>  breaks fate
>>
>>  --- ./tests/ref/fate/imgutils	2025-03-14 00:56:35.182435551 +0100
>>  +++ tests/data/fate/imgutils	2025-03-14 16:12:30.642340871 +0100
>>  @@ -296,6 +296,8 @@
>>    yaf32le         planes: 1, linesizes: 512   0   0   0, plane_sizes:
>>    24576     0     0     0, plane_offsets:     0     0     0, total_size:
>>    24576
>>    yaf16be         planes: 1, linesizes: 256   0   0   0, plane_sizes:
>>    12288     0     0     0, plane_offsets:     0     0     0, total_size:
>>    12288
>>    yaf16le         planes: 1, linesizes: 256   0   0   0, plane_sizes:
>>    12288     0     0     0, plane_offsets:     0     0     0, total_size:
>>    12288
>>  +gbrap32be       planes: 4, linesizes: 256 256 256 256, plane_sizes: 12288
>>  12288 12288 12288, plane_offsets: 12288 12288 12288, total_size: 49152
>>  +gbrap32le       planes: 4, linesizes: 256 256 256 256, plane_sizes: 12288
>>  12288 12288 12288, plane_offsets: 12288 12288 12288, total_size: 49152
>>
>>    image_fill_black tests
>>    yuv420p         total_size:   4608,  black_unknown_crc: 0xd00f6cc6,
>>    black_tv_crc: 0xd00f6cc6,  black_pc_crc: 0x234969af
>>  @@ -539,3 +541,5 @@
>>    yaf32le         total_size:  24576,  black_unknown_crc: 0xfd900236,
>>    black_tv_crc: 0xfd900236,  black_pc_crc: 0xdcaf0cb1
>>    yaf16be         total_size:  12288,  black_unknown_crc: 0x7afe9aae,
>>    black_tv_crc: 0x7afe9aae,  black_pc_crc: 0x0fc0a5d0
>>    yaf16le         total_size:  12288,  black_unknown_crc: 0x94c0068b,
>>    black_tv_crc: 0x94c0068b,  black_pc_crc: 0xc05ce449
>>  +gbrap32be       total_size:  49152,  black_unknown_crc: 0x7bd30c95,
>>  black_tv_crc: 0x7bd30c95,  black_pc_crc: 0x7bd30c95
>>  +gbrap32le       total_size:  49152,  black_unknown_crc: 0x7bd30c95,
>>  black_tv_crc: 0x7bd30c95,  black_pc_crc: 0x7bd30c95
>>  Test imgutils failed. Look at tests/data/fate/imgutils.err for details.
>>  make: *** [tests/Makefile:311: fate-imgutils] Error 1
>
> I know. Was planning on fixing fate tests when I push, since it makes 
> rebasing easier.

I don't think this is a good practice. It makes patch review or testing 
harder, the fact that a patch changes fate is useful information for 
reviewer. Not to mention that you can easily forgot to update the fate 
references before push, and a pushed commit which fails fate is a bad 
thing. So please, do not send patches which fails to pass fate.

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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