Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Tomas Härdin" <tjoppen@acc.umu.se>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support
Date: Fri, 29 Jul 2022 06:15:22 +0200
Message-ID: <76a5fe1ae85a311947340bb03b942d7f7a3cdc04.camel@acc.umu.se> (raw)
In-Reply-To: <20220728231852.GD2088045@pb2>

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
> > 
> > > +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.

> 
> 
> > 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".

  reply	other threads:[~2022-07-29  4:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-11 21:44 Michael Niedermayer
2022-07-11 21:44 ` [FFmpeg-devel] [PATCH 2/3] avformat/mpc8: Check and propagate more errors Michael Niedermayer
2022-07-21 17:38   ` Michael Niedermayer
2022-07-11 21:44 ` [FFmpeg-devel] [PATCH 3/3] tools/target_dec_fuzzer: Adjust threshold for ANM Michael Niedermayer
2022-07-12 17:57   ` Michael Niedermayer
2022-07-13 13:58 ` [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: SMPTE RDD 48:2018 support Dave Rice
2022-07-18 18:35 ` Tomas Härdin
2022-07-19 11:54   ` Michael Niedermayer
2022-07-19 13:48 ` Tomas Härdin
2022-07-28 23:18   ` Michael Niedermayer
2022-07-29  4:15     ` Tomas Härdin [this message]
2022-07-29 12:14       ` Pierre-Anthony Lemieux
2022-07-29 14:19         ` Tomas Härdin
2022-07-29 14:24           ` Pierre-Anthony Lemieux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=76a5fe1ae85a311947340bb03b942d7f7a3cdc04.camel@acc.umu.se \
    --to=tjoppen@acc.umu.se \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git