[FFmpeg-devel] [PATCH 2/6] lavf: APV demuxer

[Mark Thompson]

On 20/04/2025 17:20, James Almer wrote:
> On 4/20/2025 1:07 PM, Derek Buitenhuis wrote:
>> On 4/19/2025 8:07 PM, Mark Thompson wrote:
>>> +typedef struct APVHeaderInfo {
>>> +    uint8_t  pbu_type;
>>> +    uint16_t group_id;
>>> +
>>> +    uint8_t  profile_idc;
>>> +    uint8_t  level_idc;
>>> +    uint8_t  band_idc;
>>> +
>>> +    uint32_t frame_width;
>>> +    uint32_t frame_height;
>>> +
>>> +    uint8_t  chroma_format_idc;
>>> +    uint8_t  bit_depth_minus8;
>>> +
>>> +    enum AVPixelFormat pixel_format;
>>> +} APVHeaderInfo;
>>
>> Possibly this should put in the codec private data, and used
>> in the decoder directly, since APV in ISOBMFF requires a global
>> APVDecoderConfigurationRecord to be used - i.e. align with that.
>> Mostly because I imagine that is how most APV is produced on
>> phones.
>>
>> (Also, just excited to see APV!)
> Assuming we define extradata for this codec, given there's no "raw" version of it like there's for h26*, we might want to enforce it to always be a APVDecoderConfigurationRecord, even if exported from the raw demuxer.
> Having extradata will also let us know the value of things like bit_depth during init(), so things like DSP function pointers can be set with that knowledge in mind.

For whole-frame cases the APVDecoderConfigurationRecord is irrelevant to the decoder as all the information is in the bitstream.  It's not clear to me how the tile-subsample works, but I assume the normal bitstream is still present.

Since it is not required by the decoder, mandating extradata there seems needlessly annoying for API users - even if they just want decode they have to do something else to make the extradata.

(On the other hand, it seems fine to have an extradata there that the decoder ignores but the demuxer and encoder produce for an ISOBMFF muxer if that is the preferred model.)

Thanks,

- Mark