Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-14 Thread Matthieu Bouron
On Tue, Dec 14, 2021 at 10:41:27AM +0100, Matthieu Bouron wrote:
> On Tue, Nov 30, 2021 at 10:22:20AM +0100, Nicolas Gaullier wrote:
> > Fix GC container ul.
> > Fix GC element type both for the generic case and for OPAtom.
> > 
> > Thanks to Philip de Nier 
> > for checking the values, especially for OPAtom.
> > ---
> >  libavformat/mxfenc.c  | 8 ++--
> >  tests/ref/lavf/mxf_opatom | 2 +-
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > index fcd9afda2a..38de3d1ab5 100644
> > --- a/libavformat/mxfenc.c
> > +++ b/libavformat/mxfenc.c
> > @@ -32,6 +32,7 @@
> >   * SMPTE 379M MXF Generic Container
> >   * SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
> >   * SMPTE 422M Mapping JPEG 2000 Codestreams into the MXF Generic Container
> > + * SMPTE ST2019-4 Mapping VC-3 Coding Units into the MXF Generic Container
> >   * SMPTE RP210: SMPTE Metadata Dictionary
> >   * SMPTE RP224: Registry of SMPTE Universal Labels
> >   */
> > @@ -181,8 +182,8 @@ static const MXFContainerEssenceEntry 
> > mxf_essence_container_uls[] = {
> >{ 
> > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x00,0x00,0x00
> >  },
> >mxf_write_cdci_desc },
> >  // DNxHD
> > -{ { 
> > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x11,0x01,0x00
> >  },
> > -  { 
> > 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x05,0x00
> >  },
> > +{ { 
> > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11,0x01,0x00
> >  },
> 
> Can the chunk fixing the container UL be put in a separate commit (I just
> checked the spec and it is fine) ?
> 
> > +  { 
> > 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x0C,0x00
> >  },
> 
> It only have access to ST2019-4 (2008) and it mentions two possible value
> for the essence element type:
> - 0x05 frame wrapped picture element
> - 0x06 clip wrapped picture element
> 
> Is 0x0C introduced in a later revision of ST2019 ?

So after an offline discussion with Nicolas and Philip, it turns out, the
values for the essence element type were updated from 0x05/0x06 to
0x0C/0x0D in the 2009 revision of ST2019-4 (and they are still current as
of the latest revision of the spec).

So I'm fine with the change from 0x05 to 0x0C and the AVID hack (using
0x06 instead of 0x0D) as it is what BMX is doing. Can you please add a
comment regarding the AVID hack and why we are not using 0x0D ?

[...]

Thanks,
Matthieu
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-14 Thread Matthieu Bouron
On Tue, Nov 30, 2021 at 10:22:20AM +0100, Nicolas Gaullier wrote:
> Fix GC container ul.
> Fix GC element type both for the generic case and for OPAtom.
> 
> Thanks to Philip de Nier 
> for checking the values, especially for OPAtom.
> ---
>  libavformat/mxfenc.c  | 8 ++--
>  tests/ref/lavf/mxf_opatom | 2 +-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index fcd9afda2a..38de3d1ab5 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -32,6 +32,7 @@
>   * SMPTE 379M MXF Generic Container
>   * SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
>   * SMPTE 422M Mapping JPEG 2000 Codestreams into the MXF Generic Container
> + * SMPTE ST2019-4 Mapping VC-3 Coding Units into the MXF Generic Container
>   * SMPTE RP210: SMPTE Metadata Dictionary
>   * SMPTE RP224: Registry of SMPTE Universal Labels
>   */
> @@ -181,8 +182,8 @@ static const MXFContainerEssenceEntry 
> mxf_essence_container_uls[] = {
>{ 
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x00,0x00,0x00
>  },
>mxf_write_cdci_desc },
>  // DNxHD
> -{ { 
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x11,0x01,0x00
>  },
> -  { 
> 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x05,0x00
>  },
> +{ { 
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11,0x01,0x00
>  },

Can the chunk fixing the container UL be put in a separate commit (I just
checked the spec and it is fine) ?

> +  { 
> 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x0C,0x00
>  },

It only have access to ST2019-4 (2008) and it mentions two possible value
for the essence element type:
- 0x05 frame wrapped picture element
- 0x06 clip wrapped picture element

Is 0x0C introduced in a later revision of ST2019 ?

>{ 
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x01,0x00,0x00
>  },
>mxf_write_cdci_desc },
>  // JPEG2000
> @@ -2674,6 +2675,9 @@ static int mxf_init(AVFormatContext *s)
>  
>  memcpy(sc->track_essence_element_key, 
> mxf_essence_container_uls[sc->index].element_ul, 15);
>  sc->track_essence_element_key[15] = present[sc->index];
> +if (s->oformat == &ff_mxf_opatom_muxer && st->codecpar->codec_id == 
> AV_CODEC_ID_DNXHD) {
> +sc->track_essence_element_key[14] = 0x06;
> +}
>  PRINT_KEY(s, "track essence element key", 
> sc->track_essence_element_key);
>  
>  if (!present[sc->index])
> diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
> index 61e70b..e34cf2559e 100644
> --- a/tests/ref/lavf/mxf_opatom
> +++ b/tests/ref/lavf/mxf_opatom
> @@ -1,3 +1,3 @@
> -5d235c127ace64b1f4fe6c79a7ca8be6 *tests/data/lavf/lavf.mxf_opatom
> +aab6397829bd90f0c77a3f9fde53bb9c *tests/data/lavf/lavf.mxf_opatom
>  4717625 tests/data/lavf/lavf.mxf_opatom
>  tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a
> -- 
> 2.34.0
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-08 Thread Tomas Härdin
ons 2021-12-08 klockan 02:18 +0100 skrev Marton Balint:
> 
> 
> On Wed, 8 Dec 2021, Tomas Härdin wrote:
> 
> > fre 2021-12-03 klockan 09:38 + skrev Nicolas Gaullier:
> > > > Please add a reference to the relevant SMPTE document in the
> > > > comment, or perhaps at the list of references at the start of
> > > > the
> > > > file
> > > > 
> > > > /Tomas
> > > 
> > > I have added the reference to ST2019-4 for "VC3 mapping", so that
> > > should be ok for generic standard files.
> > > It seems redundant for me, but if you want, I could add the link
> > > to
> > > the online register where the container ul is public ?
> > > https://registry.smpte-ra.org/apps/pages/
> > > 
> > > Concerning the essence key, it is more tricky because of AVID in
> > > the
> > > place...
> > > To start with, apart AVID, all frame-wrapped samples I have (I
> > > can
> > > share them with you but not all of them publicly), do respect the
> > > standard
> > > - frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always
> > > 0x0C
> > > ("DNxHD" frame-mapping)
> > > There are up to date publicly available ARRI samples where 0x0C
> > > is
> > > used here:
> > >  
> > > https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage
> > > 
> > > But I also have an AVID Op1a file where the value 0x05 is used
> > > ("MPEG" frame-mapping, ie. s381m).
> > > And concerning OPAtom, Philip de Nier has an AVID sample where
> > > the
> > > value 0x06 is used ("MPEG" clip-wrapping).
> > > 
> > > So, what is apparent at the end is that :
> > > - apart from AVID, the standard values 0x0c/0x0d are used
> > > - AVID uses the values from the older "MPEG mapping" (ie smpte
> > > 381m)
> > > 
> > > Now :
> > > - currently ffmpeg uses 0x05 for OPatom which does not follow any
> > > implementation and seems bad
> > > - it seems there is a consensus (incl. AVID) to always use 0x05
> > > or
> > > 0x0C for frame-wrapping and 0x06 or 0x0d for clip-wrapping
> > > (OPAtom)
> > > => follow either s381m or st2019-4
> > > - it seems clear ffmpeg shall take the "standard-flavor" for
> > > generic
> > > OP's, so 0x0C for frame-based wrapping
> > > - it is less clear about OPAtom which is rather an AVID-hack-
> > > thing,
> > > but it should be moved to either 0x06 or 0x0d
> > > - I have discussed this with philip de nier, and bmx (a reference
> > > software in my opinion) will stick to the AVID form, so 0x06. And
> > > I
> > > think it is reasonable, since OPAtom/Avid are almost the same
> > > damn
> > > thing
> > > 
> > > Note: no matter the essence key, the link between the tracks and
> > > the
> > > body with the TrackNumber always work, so it seems there are not
> > > much
> > > interoperability issues with it.
> > 
> > Seems I missed the reference to the VC-3 spec somehow, sorry. Since
> > it
> > is Matthieu Bouron who added this initially, you should really talk
> > to
> > him. I care less about mxfenc than I do mxfdec, since the latter
> > can
> > more easily induce CVEs.
> > 
> > That said, if we want this to work correctly for everyone then we
> > need
> > a proper set of tests. We also need to go over all the specs, and
> > what
> > all implementations are currently doing. Which is a lot of work. Or
> > a
> > lot of billable hours!
> 
> Nicolas already made a reasonable investigation, so I am not sure
> what 
> else is required. FWIW there is also a trac ticket with this issue:
> 
> https://trac.ffmpeg.org/ticket/6380

I suppose we should just try to get a hold of Matthieu then, see if it
breaks any of his stuff

> > 
> > This is why I've said for years now that we should delete mxfenc
> > and
> > just bring in bmx. We don't need what little free software people
> > there
> > are who know MXF to keep maintaining two codebases.
> > 
> > We could just bring in bmx as a subtree and be done with it. Delete
> > all
> > MXF code native to FFmpeg, put all effort into bmx. People in this
> > project suffer from the belief that code is valuable rather than a
> > liability. Worse is not better. Correct is better.
> 
> Tested and polished code is valuable, and I think you underestimate
> the 
> work required to integrate bmxlib as an mxf muxer. If somebody starts
> working on it, great. I certainly won't oppose it to add it as an 
> alternate way to mux MXF files.

Perhaps next time I get a major MXF job I'll look into it. Because what
I don't like is the hacks upon hacks that will need to be implemented
in multiple places.

> 
> But also keep in mind that ffmpeg tends to prefer native components,
> and 
> the external library (and its integration to ffmpeg) has to be
> superior in 
> every way to even think about dropping the native one...

Of course. There must at the very least be feature parity. But I will
keep pointing out that we should aim to have less code, not more.
Especially in lavf. The NIH runs deep, and it must be combatted.

For lavc this is less clear because sometimes we get codec
implementations that are better in some aspect. 

Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-08 Thread Paul B Mahol
This is unacceptable behavior for maintainer.

On Wed, Dec 8, 2021 at 12:13 AM Tomas Härdin  wrote:

> fre 2021-12-03 klockan 09:38 + skrev Nicolas Gaullier:
> > > Please add a reference to the relevant SMPTE document in the
> > > comment, or perhaps at the list of references at the start of the
> > > file
> > >
> > > /Tomas
> >
> > I have added the reference to ST2019-4 for "VC3 mapping", so that
> > should be ok for generic standard files.
> > It seems redundant for me, but if you want, I could add the link to
> > the online register where the container ul is public ?
> > https://registry.smpte-ra.org/apps/pages/
> >
> > Concerning the essence key, it is more tricky because of AVID in the
> > place...
> > To start with, apart AVID, all frame-wrapped samples I have (I can
> > share them with you but not all of them publicly), do respect the
> > standard
> > - frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always 0x0C
> > ("DNxHD" frame-mapping)
> > There are up to date publicly available ARRI samples where 0x0C is
> > used here:
> >
> https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage
> >
> > But I also have an AVID Op1a file where the value 0x05 is used
> > ("MPEG" frame-mapping, ie. s381m).
> > And concerning OPAtom, Philip de Nier has an AVID sample where the
> > value 0x06 is used ("MPEG" clip-wrapping).
> >
> > So, what is apparent at the end is that :
> > - apart from AVID, the standard values 0x0c/0x0d are used
> > - AVID uses the values from the older "MPEG mapping" (ie smpte 381m)
> >
> > Now :
> > - currently ffmpeg uses 0x05 for OPatom which does not follow any
> > implementation and seems bad
> > - it seems there is a consensus (incl. AVID) to always use 0x05 or
> > 0x0C for frame-wrapping and 0x06 or 0x0d for clip-wrapping (OPAtom)
> > => follow either s381m or st2019-4
> > - it seems clear ffmpeg shall take the "standard-flavor" for generic
> > OP's, so 0x0C for frame-based wrapping
> > - it is less clear about OPAtom which is rather an AVID-hack-thing,
> > but it should be moved to either 0x06 or 0x0d
> > - I have discussed this with philip de nier, and bmx (a reference
> > software in my opinion) will stick to the AVID form, so 0x06. And I
> > think it is reasonable, since OPAtom/Avid are almost the same damn
> > thing
> >
> > Note: no matter the essence key, the link between the tracks and the
> > body with the TrackNumber always work, so it seems there are not much
> > interoperability issues with it.
>
> Seems I missed the reference to the VC-3 spec somehow, sorry. Since it
> is Matthieu Bouron who added this initially, you should really talk to
> him. I care less about mxfenc than I do mxfdec, since the latter can
> more easily induce CVEs.
>
> That said, if we want this to work correctly for everyone then we need
> a proper set of tests. We also need to go over all the specs, and what
> all implementations are currently doing. Which is a lot of work. Or a
> lot of billable hours!
>
> This is why I've said for years now that we should delete mxfenc and
> just bring in bmx. We don't need what little free software people there
> are who know MXF to keep maintaining two codebases.
>
> We could just bring in bmx as a subtree and be done with it. Delete all
> MXF code native to FFmpeg, put all effort into bmx. People in this
> project suffer from the belief that code is valuable rather than a
> liability. Worse is not better. Correct is better.
>
> /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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-07 Thread Marton Balint



On Wed, 8 Dec 2021, Tomas Härdin wrote:


fre 2021-12-03 klockan 09:38 + skrev Nicolas Gaullier:

> Please add a reference to the relevant SMPTE document in the
> comment, or perhaps at the list of references at the start of the
> file
> 
> /Tomas


I have added the reference to ST2019-4 for "VC3 mapping", so that
should be ok for generic standard files.
It seems redundant for me, but if you want, I could add the link to
the online register where the container ul is public ?
https://registry.smpte-ra.org/apps/pages/

Concerning the essence key, it is more tricky because of AVID in the
place...
To start with, apart AVID, all frame-wrapped samples I have (I can
share them with you but not all of them publicly), do respect the
standard
- frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always 0x0C
("DNxHD" frame-mapping)
There are up to date publicly available ARRI samples where 0x0C is
used here:
https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage

But I also have an AVID Op1a file where the value 0x05 is used
("MPEG" frame-mapping, ie. s381m).
And concerning OPAtom, Philip de Nier has an AVID sample where the
value 0x06 is used ("MPEG" clip-wrapping).

So, what is apparent at the end is that :
- apart from AVID, the standard values 0x0c/0x0d are used
- AVID uses the values from the older "MPEG mapping" (ie smpte 381m)

Now :
- currently ffmpeg uses 0x05 for OPatom which does not follow any
implementation and seems bad
- it seems there is a consensus (incl. AVID) to always use 0x05 or
0x0C for frame-wrapping and 0x06 or 0x0d for clip-wrapping (OPAtom)
=> follow either s381m or st2019-4
- it seems clear ffmpeg shall take the "standard-flavor" for generic
OP's, so 0x0C for frame-based wrapping
- it is less clear about OPAtom which is rather an AVID-hack-thing,
but it should be moved to either 0x06 or 0x0d
- I have discussed this with philip de nier, and bmx (a reference
software in my opinion) will stick to the AVID form, so 0x06. And I
think it is reasonable, since OPAtom/Avid are almost the same damn
thing

Note: no matter the essence key, the link between the tracks and the
body with the TrackNumber always work, so it seems there are not much
interoperability issues with it.


Seems I missed the reference to the VC-3 spec somehow, sorry. Since it
is Matthieu Bouron who added this initially, you should really talk to
him. I care less about mxfenc than I do mxfdec, since the latter can
more easily induce CVEs.

That said, if we want this to work correctly for everyone then we need
a proper set of tests. We also need to go over all the specs, and what
all implementations are currently doing. Which is a lot of work. Or a
lot of billable hours!


Nicolas already made a reasonable investigation, so I am not sure what 
else is required. FWIW there is also a trac ticket with this issue:


https://trac.ffmpeg.org/ticket/6380



This is why I've said for years now that we should delete mxfenc and
just bring in bmx. We don't need what little free software people there
are who know MXF to keep maintaining two codebases.

We could just bring in bmx as a subtree and be done with it. Delete all
MXF code native to FFmpeg, put all effort into bmx. People in this
project suffer from the belief that code is valuable rather than a
liability. Worse is not better. Correct is better.


Tested and polished code is valuable, and I think you underestimate the 
work required to integrate bmxlib as an mxf muxer. If somebody starts 
working on it, great. I certainly won't oppose it to add it as an 
alternate way to mux MXF files.


But also keep in mind that ffmpeg tends to prefer native components, and 
the external library (and its integration to ffmpeg) has to be superior in 
every way to even think about dropping the native one...


Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-07 Thread Tomas Härdin
fre 2021-12-03 klockan 09:38 + skrev Nicolas Gaullier:
> > Please add a reference to the relevant SMPTE document in the
> > comment, or perhaps at the list of references at the start of the
> > file
> > 
> > /Tomas
> 
> I have added the reference to ST2019-4 for "VC3 mapping", so that
> should be ok for generic standard files.
> It seems redundant for me, but if you want, I could add the link to
> the online register where the container ul is public ?
> https://registry.smpte-ra.org/apps/pages/
> 
> Concerning the essence key, it is more tricky because of AVID in the
> place...
> To start with, apart AVID, all frame-wrapped samples I have (I can
> share them with you but not all of them publicly), do respect the
> standard
> - frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always 0x0C
> ("DNxHD" frame-mapping)
> There are up to date publicly available ARRI samples where 0x0C is
> used here:
> https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage
> 
> But I also have an AVID Op1a file where the value 0x05 is used
> ("MPEG" frame-mapping, ie. s381m).
> And concerning OPAtom, Philip de Nier has an AVID sample where the
> value 0x06 is used ("MPEG" clip-wrapping).
> 
> So, what is apparent at the end is that :
> - apart from AVID, the standard values 0x0c/0x0d are used
> - AVID uses the values from the older "MPEG mapping" (ie smpte 381m)
> 
> Now :
> - currently ffmpeg uses 0x05 for OPatom which does not follow any
> implementation and seems bad
> - it seems there is a consensus (incl. AVID) to always use 0x05 or
> 0x0C for frame-wrapping and 0x06 or 0x0d for clip-wrapping (OPAtom)
> => follow either s381m or st2019-4
> - it seems clear ffmpeg shall take the "standard-flavor" for generic
> OP's, so 0x0C for frame-based wrapping
> - it is less clear about OPAtom which is rather an AVID-hack-thing,
> but it should be moved to either 0x06 or 0x0d
> - I have discussed this with philip de nier, and bmx (a reference
> software in my opinion) will stick to the AVID form, so 0x06. And I
> think it is reasonable, since OPAtom/Avid are almost the same damn
> thing
> 
> Note: no matter the essence key, the link between the tracks and the
> body with the TrackNumber always work, so it seems there are not much
> interoperability issues with it.

Seems I missed the reference to the VC-3 spec somehow, sorry. Since it
is Matthieu Bouron who added this initially, you should really talk to
him. I care less about mxfenc than I do mxfdec, since the latter can
more easily induce CVEs.

That said, if we want this to work correctly for everyone then we need
a proper set of tests. We also need to go over all the specs, and what
all implementations are currently doing. Which is a lot of work. Or a
lot of billable hours!

This is why I've said for years now that we should delete mxfenc and
just bring in bmx. We don't need what little free software people there
are who know MXF to keep maintaining two codebases.

We could just bring in bmx as a subtree and be done with it. Delete all
MXF code native to FFmpeg, put all effort into bmx. People in this
project suffer from the belief that code is valuable rather than a
liability. Worse is not better. Correct is better.

/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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-03 Thread Nicolas Gaullier
>Please add a reference to the relevant SMPTE document in the comment, or 
>perhaps at the list of references at the start of the file
>
>/Tomas

I have added the reference to ST2019-4 for "VC3 mapping", so that should be ok 
for generic standard files.
It seems redundant for me, but if you want, I could add the link to the online 
register where the container ul is public ?
https://registry.smpte-ra.org/apps/pages/

Concerning the essence key, it is more tricky because of AVID in the place...
To start with, apart AVID, all frame-wrapped samples I have (I can share them 
with you but not all of them publicly), do respect the standard
- frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always 0x0C ("DNxHD" 
frame-mapping)
There are up to date publicly available ARRI samples where 0x0C is used here:
https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage

But I also have an AVID Op1a file where the value 0x05 is used ("MPEG" 
frame-mapping, ie. s381m).
And concerning OPAtom, Philip de Nier has an AVID sample where the value 0x06 
is used ("MPEG" clip-wrapping).

So, what is apparent at the end is that :
- apart from AVID, the standard values 0x0c/0x0d are used
- AVID uses the values from the older "MPEG mapping" (ie smpte 381m)

Now :
- currently ffmpeg uses 0x05 for OPatom which does not follow any 
implementation and seems bad
- it seems there is a consensus (incl. AVID) to always use 0x05 or 0x0C for 
frame-wrapping and 0x06 or 0x0d for clip-wrapping (OPAtom) => follow either 
s381m or st2019-4
- it seems clear ffmpeg shall take the "standard-flavor" for generic OP's, so 
0x0C for frame-based wrapping
- it is less clear about OPAtom which is rather an AVID-hack-thing, but it 
should be moved to either 0x06 or 0x0d
- I have discussed this with philip de nier, and bmx (a reference software in 
my opinion) will stick to the AVID form, so 0x06. And I think it is reasonable, 
since OPAtom/Avid are almost the same damn thing

Note: no matter the essence key, the link between the tracks and the body with 
the TrackNumber always work, so it seems there are not much interoperability 
issues with it.

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

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


Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-12-01 Thread Tomas Härdin
tis 2021-11-30 klockan 10:22 +0100 skrev Nicolas Gaullier:
Fix GC container ul.
Fix GC element type both for the generic case and for OPAtom.

Thanks to Philip de Nier 
for checking the values, especially for OPAtom.
---
 libavformat/mxfenc.c  | 8 ++--
 tests/ref/lavf/mxf_opatom | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index fcd9afda2a..38de3d1ab5 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -32,6 +32,7 @@
  * SMPTE 379M MXF Generic Container
  * SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
  * SMPTE 422M Mapping JPEG 2000 Codestreams into the MXF Generic
Container
+ * SMPTE ST2019-4 Mapping VC-3 Coding Units into the MXF Generic
Container
  * SMPTE RP210: SMPTE Metadata Dictionary
  * SMPTE RP224: Registry of SMPTE Universal Labels
  */
@@ -181,8 +182,8 @@ static const MXFContainerEssenceEntry
mxf_essence_container_uls[] = {
   {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x00,0
x00,0x00 },
   mxf_write_cdci_desc },
 // DNxHD
-    { {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x11,0
x01,0x00 },
-  {
0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0
x05,0x00 },
+    { {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11,0
x01,0x00 },
+  {
0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0
x0C,0x00 },
   {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x01,0
x00,0x00 },

Please add a reference to the relevant SMPTE document in the comment,
or perhaps at the list of references at the start of the file

/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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs

2021-11-30 Thread Nicolas Gaullier
Fix GC container ul.
Fix GC element type both for the generic case and for OPAtom.

Thanks to Philip de Nier 
for checking the values, especially for OPAtom.
---
 libavformat/mxfenc.c  | 8 ++--
 tests/ref/lavf/mxf_opatom | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index fcd9afda2a..38de3d1ab5 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -32,6 +32,7 @@
  * SMPTE 379M MXF Generic Container
  * SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
  * SMPTE 422M Mapping JPEG 2000 Codestreams into the MXF Generic Container
+ * SMPTE ST2019-4 Mapping VC-3 Coding Units into the MXF Generic Container
  * SMPTE RP210: SMPTE Metadata Dictionary
  * SMPTE RP224: Registry of SMPTE Universal Labels
  */
@@ -181,8 +182,8 @@ static const MXFContainerEssenceEntry 
mxf_essence_container_uls[] = {
   { 
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x00,0x00,0x00 
},
   mxf_write_cdci_desc },
 // DNxHD
-{ { 
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x11,0x01,0x00 
},
-  { 
0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x05,0x00 
},
+{ { 
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11,0x01,0x00 
},
+  { 
0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x0C,0x00 
},
   { 
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x01,0x00,0x00 
},
   mxf_write_cdci_desc },
 // JPEG2000
@@ -2674,6 +2675,9 @@ static int mxf_init(AVFormatContext *s)
 
 memcpy(sc->track_essence_element_key, 
mxf_essence_container_uls[sc->index].element_ul, 15);
 sc->track_essence_element_key[15] = present[sc->index];
+if (s->oformat == &ff_mxf_opatom_muxer && st->codecpar->codec_id == 
AV_CODEC_ID_DNXHD) {
+sc->track_essence_element_key[14] = 0x06;
+}
 PRINT_KEY(s, "track essence element key", 
sc->track_essence_element_key);
 
 if (!present[sc->index])
diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
index 61e70b..e34cf2559e 100644
--- a/tests/ref/lavf/mxf_opatom
+++ b/tests/ref/lavf/mxf_opatom
@@ -1,3 +1,3 @@
-5d235c127ace64b1f4fe6c79a7ca8be6 *tests/data/lavf/lavf.mxf_opatom
+aab6397829bd90f0c77a3f9fde53bb9c *tests/data/lavf/lavf.mxf_opatom
 4717625 tests/data/lavf/lavf.mxf_opatom
 tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a
-- 
2.34.0

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

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