Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Pierre-Anthony Lemieux <pal@sandflow.com>]

On Fri, Jul 29, 2022 at 6:15 AM Tomas Härdin <tjoppen@acc.umu.se> wrote:
>
> fre 2022-07-29 klockan 01:18 +0200 skrev Michael Niedermayer:
> > On Tue, Jul 19, 2022 at 03:48:59PM +0200, Tomas Härdin wrote:
> > > mån 2022-07-11 klockan 23:44 +0200 skrev Michael Niedermayer:
> > > >
> > > > +    { {
> > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,
> > > > 0x09
> > > > ,0x01,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V0 */
> > > > +    { {
> > > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,
> > > > 0x09
> > > > ,0x02,0x00 }, 15,       AV_CODEC_ID_FFV1 }, /*FFV1 V1 */
> > > > +    { {
> > >
> > > Double-checked, these are correct

I recommend the draft SMPTE metadata registry at the following as the
reference for ULs:

https://registry.smpte-ra.org/apps/pages/

The registry is kept up-to-date, machine readable and free to access.

> > >
> > > > +typedef struct MXFFFV1SubDescriptor {
> > > > +    MXFMetadataSet meta;
> > > > +    uint8_t *extradata;
> > > > +    int extradata_size;
> > >
> > > Is FFV1 extradata size bounded? It so we could avoid an allocation.
> > > Either way the local set syntax limits this to 64k, see below.
> >
> > the extradata is extensible so future versions can be bigger.
> > For the current version there should be a maximum. As the extradata
> > uses
> > an adaptive range coder it is not trivial to give a tight limit. It
> > would
> > be easy to give some non tight limit. But iam not sure this has any
> > point
> > as future versions can be bigger
> > [...]
> > i also dont think a static array is a good idea, there is
> > no size limit unless you want to limit to a specific version and
> > compute a worst case bound on a adaptive coder. And then
> > that worst case would be orders of magnitude bigger than real
> > extradata
> > because real extradata compresses quite well. While the worst case
> > would
> > be the case that is biggest and compresses worst. So a static array
> > would waste space
>
> Values in (0x53) local sets are limited to 64k, so it should be fine in
> this context
>
>
> > >
> > > > +    { {
> > > > 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,
> > > > 0x01
> > > > ,0x81,0x03 }, mxf_read_ffv1_sub_descriptor,
> > > > sizeof(MXFFFV1SubDescriptor), FFV1SubDescriptor },
> > >
> > > The spec says 0x7F not 0x53. 0x53 is used in groups with 2-byte
> > > tags
> >
> > If i put 0x7F with no other change there, it will break demuxing the
> > files i have
> > I guess i must have copied this from the files without noticing it
> > mismatches
> > the spec
>
> Yeah I would expect it to break with 0x7F. Perhaps this will change
> when the spec becomes official. If you have contact with the people
> involved in this then I suggest asking them about this. It could also
> be a typo in the spec.

Byte 6 of Group ULs is set by convention to the wildcard value 0x7F to
indicate that the encoding of the Group is not limited to 0x53 (local
set with 2-byte local tags and length field). See the following:

https://registry.smpte-ra.org/view/draft/docs/Submissions%20Overview/Document-Editors-Information--Style-Guide/#groups-ul

MXF restricts header metadata to local sets with 2-byte local tags and
2-byte or BER lengths, so byte 6 can be 0x13 or 0x53.

>
> >
> >
> > > rather than full KLVs. The intent here seems to be to use local
> > > tags,
> > > which fortuitously limits extradata_size to 64k. This makes me
> > > think
> > > Amendment 1:2022 is wrong or that 0x7F is just to signal private
> > > use
> > > until it gets rolled into the next version of RDD 48.
> > >
> > > Tables 18 and 23 in S377m-1-2009 say that 0x7F corresponds to
> > > "Abstract
> > > Groups" which are "never encoded as Metadata Sets".
> > >
> > > Reading S336m-2007 it seems one can actually use various lengths
> > > and
> > > tag sizes. 0x53 corresponds to 2 byte length and 2 byte tag. S377m
> > > says
> > > that in addition to this, 0x13 is allowed in MXF which uses ASN.1
> > > BER
> > > encoded lengths. Don't know if any files in the wild use that.
> > > Probably
> > > not.
> >
> > couldnt they make this more complex and bizarr?
>
> Welcome to the world of MXF.
>
> The flip side here is that extradata over 64k *can* be encoded legally,
> but I have never seen it in the wild and mxfdec doesn't support it
> (yet). In that light it makes sense to keep the extradata allocation
> dynamic
>
> /Tomas
>
> _______________________________________________
> 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".
_______________________________________________
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".