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" <git@haerdin.se>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] lavf/mxfenc: Bump EDIT_UNITS_PER_BODY
Date: Tue, 14 Feb 2023 10:19:08 +0100
Message-ID: <6f7243ced0cd5059f51fe360021e48d7f3a2551c.camel@haerdin.se> (raw)
In-Reply-To: <9da1fcb9-4a7f-89dc-b337-311737829948@passwd.hu>

mån 2023-02-13 klockan 22:05 +0100 skrev Marton Balint:
> 
> I think we have a pretty good idea that MPEG2 in MXF usually means
> some 
> broadcast realted use, therefore intent of RDD9 compliance by default
> is 
> not insane at all for MPEG2 essence. Can we please keep it as default
> for 
> MPEG2?

If the intent is to follow RDD9 then more things need to be done, among
them erroring out if gop_size > 15. EDIT_UNITS_PER_BODY is also too big
for 24/1.001 Hz essence. Finally the logic around line 2988 is wrong
since it will easily and consistently writes index table segments > 10
seconds:

> if (!mxf->edit_unit_byte_count &&
>     (!mxf->edit_units_count || mxf->edit_units_count >
> EDIT_UNITS_PER_BODY) &&
>     !(ie.flags & 0x33)) { // I-frame, GOP start
> 

This will only work for NTSC because (250+15)/30*1.001 < 8.9s
assuming gop_size <= 15, and it will be > 10s for PAL and 24/1.001 Hz.
The reallocation logic is likely there to compensate for this
wrongness.

The simplest way to remain compliant should therefore be:

* check that gop_size <= 15
* change the above condition to mxf->edit_units_count > 239-gop_size
(maybe -1)

The simplest for the latter is just > 223. If ever the muxer gets a
series of packages that then exceeds the limits set out in RDD 9-2006
then it should complain loudly and terminate so that users don't
accidentally write non-compliant files.

For the allocation stuff, we should make room for 301 EditUnits. If
ever the muxer finds the need to insert a 302nd EditUnit when muxing
MPEG2 then it should error out.

Of course a lot of this could likely be avoided if we just used BMX
instead.

> 
> > 
> > > 
> > > There is also code which allocates the index entries in 
> > > EDIT_UNITS_PER_BODY increments, that probably should be replaced
> > > with
> > > av_dynarray2_add...
> > 
> > That (re)allocation happens at most twice for assuming GOP size <
> > EDIT_UNITS_PER_BODY
> > 
> > The reason I bring this all up is because opening MXF files muxed
> > by
> > lavf over HTTP is slow as hell. This will be true for any other
> > storage
> > where seeking incurs a non-trivial cost. HDDs come to mind. Perhaps
> > that is really an mxfdec problem, but then parsing becomes even
> > hairier
> > than it already is. We'd have to binary search the file when
> > seeking,
> > hoping to find the necessary index table segments on-the-fly..
> 
> A sane muxer duplicates all index table segments in the footer, so a
> smart 
> demuxer can read the whole index from there.

True, though that requires using a dynarray. It also requires smarts
that I don't think mxfdec currently has.

> Yes, mxfdec problem. I have a 
> WIP patch somewhere fixing that.

That would be lovely

/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:[~2023-02-14  9:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13  9:58 Tomas Härdin
2023-02-13 19:33 ` Marton Balint
2023-02-13 20:45   ` Tomas Härdin
2023-02-13 21:05     ` Marton Balint
2023-02-14  9:19       ` Tomas Härdin [this message]

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=6f7243ced0cd5059f51fe360021e48d7f3a2551c.camel@haerdin.se \
    --to=git@haerdin.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