[FFmpeg-devel] [PATCH 12/12] lavf/framecrcenc: do not hash side data

[James Almer]

On 5/14/2021 5:01 AM, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2021-05-10 16:06:01)
>> On Sun, Apr 25, 2021 at 09:03:20AM +0200, Anton Khirnov wrote:
>>> There are no guarantees that all side data types have the same
>>> representation on all platforms.
>>
>>> @@ -65,63 +51,6 @@ static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>>>                pkt->stream_index, pkt->dts, pkt->pts, pkt->duration, pkt->size, crc);
>>>       if (pkt->flags != AV_PKT_FLAG_KEY)
>>>           av_strlcatf(buf, sizeof(buf), ", F=0x%0X", pkt->flags);
>>> -    if (pkt->side_data_elems) {
>>> -        int i;
>>> -        av_strlcatf(buf, sizeof(buf), ", S=%d", pkt->side_data_elems);
>>
>> The number and type of the side data elements should not be affected
>> by the problem described.
>> Maybe we could leave that. Bugs could manifest as the absence or addition
>> of side data, seeing that in framecrc_write_packet() output may be usefull
> 
> No strong opinion here - patches welcome I guess.

I can do it, but it will be a "breaking" change, assuming there are 
parsers that read the output of this muxer.
Right now you removed all the trailing properties, which while also 
breaking, a sane parser made for the old output can simply assume that 
the line was truncated. But if we re-add the side data amount and sizes 
for each of them without the hashes, things can get parsed the wrong way.

Namely, it used to be:
> 0,          0,          0,       16,    57008, 0x43416399, S=2,        8, 0x08e5014f,       88, 0xd65a04db

And now it will be something like:
> 0,          0,          0,       16,    57008, 0x43416399, S=2,        8,       88

So the question is, do we care about this? The framehash/framemd5 muxer 
prints a version number, which lets us make breaking additions parsers 
can then properly handle. Should we then just consider that parsing 
framecrc output is not a supported scenario?