[FFmpeg-devel] [PATCH v1 5/8] avformat/mov_muxer: Extended MOV muxer to handle APV video content

[Mark Thompson]

On 23/04/2025 15:13, Dawid Kozinski wrote:
> - Changes in mov_write_video_tag function to handle APV elementary stream
> - Provided structure APVDecoderConfigurationRecord that specifies the decoder configuration information for APV video content
> 
> Signed-off-by: Dawid Kozinski <d.kozinski at samsung.com>
> ---
>  libavformat/Makefile    |   2 +-
>  libavformat/apv.c       | 827 ++++++++++++++++++++++++++++++++++++++++
>  libavformat/apv.h       |  94 +++++
>  libavformat/isom_tags.c |   2 +
>  libavformat/movenc.c    |  47 +++
>  5 files changed, 971 insertions(+), 1 deletion(-)

Hi,

Two thoughts here:

First, your AVPackets contain a raw_bitstream_access_unit().  I don't think this is the right approach - the packets should contain the codec data only, not the additional encapsulation.  (This is the method I followed.)

For this patch in particular, I think it results in writing the files incorrectly: the specification says "each sample shall contain one and only one access unit of APV coded data", which I interpret to mean one access_unit() syntax structure.

This also results in the size effectively appearing multiple times in the file for no good reason:

00000020  66 72 65 65 00 01 15 db  6d 64 61 74 00 01 15 cf  |free....mdat....|

                      ^ mdat size              ^ au_size

00000030  61 50 76 31 00 01 15 c7  01 00 01 00 21 21 40 00  |aPv1........!!@.|
          ^ signature ^ pbu_size   ^ pbu_type followed by header

The separate pbu_size makes sense if there is also metadata, but having the mdat box with a size immediately followed by the same size (well, minus twelve for mdat size + mdat + au size) again inside the box does not seem helpful.

Second, I think we need a consistent decision on what the extradata should be doing.  The APVDecoderConfigurationRecord makes sense as a thing for it to contain, but it's not clear to me that it needs to exist at all as it has no effect on anything inside ffmpeg (a decoder will always ignore it).

You currently make extradata from one of your demuxers but not other one or the encoder, and nothing requires it when consuming.  Why is it useful to have ever?

Thanks,

- Mark