[FFmpeg-devel] [PATCH v3] avformat/framecrcenc: compute the checksum for side data

[James Almer]

On 5/4/2024 10:45 PM, Michael Niedermayer wrote:
> On Sat, May 04, 2024 at 06:02:25PM -0300, James Almer wrote:
>>
>>
>> On 5/4/2024 5:58 PM, Michael Niedermayer wrote:
>>> On Sat, May 04, 2024 at 12:16:00PM -0300, James Almer wrote:
>>>> On 5/4/2024 5:34 AM, Marton Balint wrote:
>>>>>
>>>>>
>>>>> On Thu, 2 May 2024, James Almer wrote:
>>>>>
>>>>>> On 5/2/2024 6:23 PM, Marton Balint wrote:
>>>>>>>
>>>>>>>
>>>>>>>    On Wed, 1 May 2024, Michael Niedermayer wrote:
>>>>>>>
>>>>>>>>    This allows detecting issues in side data related code, same as what
>>>>>>>>    framecrc does for before already for packet data itself.
>>>>>>>>
>>>>>>>>    This basically reverts c6ae560a18d67b9ddaa25a0338b7fb55e3312e57.
>>>>>>>
>>>>>>>    Can you at least add an option which allows disabling dumping the side
>>>>>>>    data? Changing the format of framecrc output again and again is
>>>>>>> not very
>>>>>>
>>>>>> The framehash/framemd5 muxer is versioned, which is what you should
>>>>>> use if you want parseable output.
>>>>>
>>>>> Okay, but then the question is that why framecrc is using different code
>>>>> and options?
>>>>
>>>> Originally it was framecrc (using AVAdler) and framemd5 (using AVMD5). The
>>>> latter was renamed/aliased to framehash and made to use the AVHash API,
>>>> which supports all lavu hashing algorithms, and is versioned.
>>>> If anyone cared, framecrc could be also made into an alias of framehash that
>>>> defaults to adler32 output, but it would result in a massive change to
>>>> reference files, if anything because AVHash initializes adler32 with a 1
>>>> whereas framecrc does it with a 0.
>>>
>>> normally starting adler32 at 0 is bad as one could prefix by a 0 byte with
>>> no checksum change, but given we also show the size that spcific case isnt
>>> an issue
>>>
>>> can we make the initial value for adler32 configureable so as to improve long
>>> term stability of checksums ? (or add a adler32_1 to AVHash)
>>
>> av_hash_init() already initializes adler32 with a 1. No need to do anything
>> there. It's the framecrc muxer that uses adler32 with 0 as init value.
> 
> i was thinking about av_hash_init() supporting a 0 init to avoid changing every
> checksum. Which would
> increase git repository size
> and make long term comparissions harder because framecrc would not be comparable
> at all. not just some extra fields that occasionally appear

Adding AVOptions is not backwards compatible, as it would only work on 
AVHashContext from the introductory commit onwards, so the only 
alternative is probably a new init function that takes an input seed 
argument. Just not an uint32_t, as that's not extensible. It would need 
to be an uint8_t array.