[FFmpeg-devel] [PATCH 1/8] avutil: introduce an Immersive Audio Model and Formats API

[James Almer]

On 12/18/2023 8:04 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-12-14 21:14:26)
>> +/**
>> + * Mix Gain Parameter Data as defined in section 3.8.1 of IAMF.
>> + */
>> +typedef struct AVIAMFMixGain {
>> +    const AVClass *av_class;
>> +
>> +    /**
>> +     * Duration for the given subblock. It must not be 0.
> 
> In what units? Same for all durations in this patch.

parameter_rate. Amended.

> 
>> +typedef struct AVIAMFParamDefinition {
>> +    const AVClass *av_class;
>> +
>> +    /**
>> +     * Offset in bytes from the start of this struct, at which the subblocks
>> +     * array is located.
>> +     */
>> +    size_t subblocks_offset;
>> +    /**
>> +     * Size in bytes of each element in the subblocks array.
>> +     */
>> +    size_t subblock_size;
>> +    /**
>> +     * Number of subblocks in the array.
>> +     *
>> +     * Must be 0 if @ref constant_subblock_duration is not 0.

Removed this line as it's bogus.

>> +     */
>> +    unsigned int nb_subblocks;
>> +
>> +    /**
>> +     * Parameters type. Determines the type of the subblock elements.
>> +     */
>> +    enum AVIAMFParamDefinitionType type;
>> +
>> +    /**
>> +     * Identifier for the paremeter substream.
>> +     */
>> +    unsigned int parameter_id;
>> +    /**
>> +     * Sample rate for the paremeter substream. It must not be 0.
>> +     */
>> +    unsigned int parameter_rate;
>> +
>> +    /**
>> +     * The duration of the all subblocks in this parameter definition.
>> +     *
>> +     * May be 0, in which case all duration values should be specified in
>> +     * another parameter definition referencing the same parameter_id.
>> +     */
>> +    unsigned int duration;
>> +    /**
>> +     * The duration of every subblock in the case where all subblocks, with
>> +     * the optional exception of the last subblock, have equal durations.
>> +     *
>> +     * Must be 0 if subblocks have different durations.
>> +     */
>> +    unsigned int constant_subblock_duration;
> 
> This also seems like should be a flags field.

No, duration and subblock duration are not the same thing. The former is 
the accumulated duration of all subblocks in a given parameter 
definition. subblock durations can be smaller, and only if they are 
constant will constant_subblock_duration be set to a value other than 0.

> 
> Otherwise looks good.
> 

Pushed. Thanks a lot for looking at it.