[FFmpeg-devel] [PATCH] avformat/framecrcenc: support calculating checksum of IAMF side data

[James Almer]

On 5/2/2024 12:16 PM, Andreas Rheinhardt wrote:
> James Almer:
>> The total allocated size for types is arch dependent, so instead calculate a
>> checksum of the fields only.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> Depends on "[PATCH v3] avformat/framecrcenc: compute the checksum for side data"
>>
>>   libavformat/framecrcenc.c | 76 ++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 71 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/framecrcenc.c b/libavformat/framecrcenc.c
>> index 8cc4a93053..6d46b51489 100644
>> --- a/libavformat/framecrcenc.c
>> +++ b/libavformat/framecrcenc.c
>> @@ -24,6 +24,7 @@
>>   #include "config.h"
>>   #include "libavutil/adler32.h"
>>   #include "libavutil/avstring.h"
>> +#include "libavutil/iamf.h"
>>   #include "libavutil/intreadwrite.h"
>>   
>>   #include "libavcodec/codec_id.h"
>> @@ -76,7 +77,8 @@ static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>>           for (int i = 0; i < pkt->side_data_elems; i++) {
>>               const AVPacketSideData *const sd = &pkt->side_data[i];
>>               const uint8_t *data = sd->data;
>> -            int64_t side_data_crc = 0;
>> +            size_t size = sd->size;
>> +            uint32_t side_data_crc = 0;
>>   
>>               switch (sd->type) {
>>   #if HAVE_BIGENDIAN
>> @@ -127,12 +129,76 @@ static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>>               case AV_PKT_DATA_IAMF_MIX_GAIN_PARAM:
>>               case AV_PKT_DATA_IAMF_DEMIXING_INFO_PARAM:
>>               case AV_PKT_DATA_IAMF_RECON_GAIN_INFO_PARAM:
>> -                side_data_crc = -1;
>> +            {
>> +                const AVIAMFParamDefinition *param = (AVIAMFParamDefinition *)sd->data;
>> +                uint8_t buf[8];
>> +                ptrdiff_t offset = 0;
>> +                size = 0;
>> +#define READ_UINT32(struct, parent, child) do { \
>> +    if ((offset + offsetof(struct, child) + sizeof(parent->child)) > sd->size) \
>> +        goto iamf_end; \
>> +    AV_WL32(buf, parent->child); \
>> +    side_data_crc = av_adler32_update(side_data_crc, buf, 4); \
>> +    size += 4; \
>> +} while (0)
>> +#define READ_RATIONAL(struct, parent, child) do { \
>> +    if ((offset + offsetof(struct, child) + sizeof(parent->child)) > sd->size) \
>> +        goto iamf_end; \
>> +    AV_WL32(buf + 0, parent->child.num); \
>> +    AV_WL32(buf + 4, parent->child.den); \
>> +    side_data_crc = av_adler32_update(side_data_crc, buf, 8); \
>> +    size += 8; \
>> +} while (0)
>> +                READ_UINT32(AVIAMFParamDefinition, param, nb_subblocks);
>> +                READ_UINT32(AVIAMFParamDefinition, param, type);
>> +                READ_UINT32(AVIAMFParamDefinition, param, parameter_id);
>> +                READ_UINT32(AVIAMFParamDefinition, param, parameter_rate);
>> +                READ_UINT32(AVIAMFParamDefinition, param, duration);
>> +                READ_UINT32(AVIAMFParamDefinition, param, constant_subblock_duration);
>> +                for (unsigned int i = 0; i < param->nb_subblocks; i++) {
>> +                    void *subblock = av_iamf_param_definition_get_subblock(param, i);
>> +
>> +                    offset = (intptr_t)subblock - (intptr_t)sd->data;
>> +                    switch (param->type) {
>> +                    case AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN: {
>> +                        const AVIAMFMixGain *mix = subblock;
>> +                        READ_UINT32(AVIAMFMixGain, mix, subblock_duration);
>> +                        READ_UINT32(AVIAMFMixGain, mix, animation_type);
>> +                        READ_RATIONAL(AVIAMFMixGain, mix, start_point_value);
>> +                        READ_RATIONAL(AVIAMFMixGain, mix, end_point_value);
>> +                        READ_RATIONAL(AVIAMFMixGain, mix, control_point_value);
>> +                        READ_RATIONAL(AVIAMFMixGain, mix, control_point_relative_time);
>> +                        break;
>> +                    }
>> +                    case AV_IAMF_PARAMETER_DEFINITION_DEMIXING: {
>> +                        const AVIAMFDemixingInfo *demix = subblock;
>> +                        READ_UINT32(AVIAMFDemixingInfo, demix, subblock_duration);
>> +                        READ_UINT32(AVIAMFDemixingInfo, demix, dmixp_mode);
>> +                        break;
>> +                    }
>> +                    case AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN: {
>> +                        const AVIAMFReconGain *recon = subblock;
>> +                        READ_UINT32(AVIAMFReconGain, recon, subblock_duration);
>> +                        if ((offset + offsetof(AVIAMFReconGain, recon_gain)
>> +                                    + sizeof(recon->recon_gain)) > sd->size)
>> +                            goto iamf_end;
>> +                        side_data_crc = av_adler32_update(side_data_crc,
>> +                                                          (uint8_t *)recon->recon_gain,
>> +                                                          sizeof(recon->recon_gain));
>> +                        size += sizeof(recon->recon_gain);
>> +                        break;
>> +                    }
>> +                    default:
>> +                        break;
>> +                    }
>> +                }
>> +                iamf_end:
>> +                break;
>> +            }
>>               }
>>   
>> -            av_strlcatf(buf, sizeof(buf), ", T=%2d, %8"SIZE_SPECIFIER, pkt->side_data[i].type, pkt->side_data[i].size);
>> -            if (side_data_crc >= 0)
>> -                av_strlcatf(buf, sizeof(buf), ", 0x%08"PRIx32, (uint32_t)side_data_crc);
>> +            av_strlcatf(buf, sizeof(buf), ", T=%2d, %8"SIZE_SPECIFIER, pkt->side_data[i].type, size);
>> +            av_strlcatf(buf, sizeof(buf), ", 0x%08"PRIx32, side_data_crc);
>>           }
>>       }
>>       av_strlcatf(buf, sizeof(buf), "\n");
> 
> This is overkill; tests for IAMF side data should use ffprobe.

This is adding support to these three side data types in framecrc 
regardless of host arch, because of the nature of the structs. The IAMF 
tests already use ffprobe.