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

[James Almer <jamrial@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2865 bytes --]

On 4/20/2025 1:57 PM, Mark Thompson wrote:
> 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
If a configuration record is already present in codecpar->extradata, 
then muxing could be possible without setting delay_moov.
The mp4 muxer could generate the APVDecoderConfigurationRecord just fine 
too with information present in the first packet, in theory, unless the 
required number_of_configuration_entry amount of entries (each for a 
different value of pbu_type) can't be made without more than one packet. 
It's not too clear for me after a quick look, and there appears to be 
non-primary frames the decoder you wrote doesn't support yet.

Encoders and demuxers are the two types of modules that would ideally 
export extradata, and decoders and muxers are two that would use it if 
present, but not depend on it.
The extract_extradata bsf can also generate it instead of having the raw 
demuxer do it.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".