| Message ID | 20240910140603.741345-4-ms+git@mur.at |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v4,1/4] libavcodec/dnxuc_parser: DNxUncompressed essence parser | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | fail | Make failed |
| andriy/make_x86 | fail | Make failed |
tis 2024-09-10 klockan 16:06 +0200 skrev Martin Schitter: > --- > libavformat/mxf.c | 1 + > libavformat/mxfdec.c | 1 + > 2 files changed, 2 insertions(+) Commit message could be better, something like "Add DNxUncompressed ULs" > diff --git a/libavformat/mxf.c b/libavformat/mxf.c > index a73e40e..35fb73e 100644 > --- a/libavformat/mxf.c > +++ b/libavformat/mxf.c > @@ -61,6 +61,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = { > { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x71,0x00,0x00,0x00 }, 13, AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */ > { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x03,0x02,0x00,0x00 }, 14, AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */ > { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x04,0x01,0x00 }, 16, AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD Legacy Avid Media Composer MXF */ > + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x1E,0x01,0x00 }, 16, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ Are really all 16 bytes significant? > { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x32,0x00,0x00 }, 14, AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC Intra */ > { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x31,0x11,0x01 }, 14, AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC SPS/PPS in-band */ > { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02,0x02,0x01 }, 16, AV_CODEC_ID_V210 }, /* V210 */ > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index ac63c0d..142b3e6 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -1597,6 +1597,7 @@ static const MXFCodecUL mxf_picture_essence_container_uls[] = { > { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00 }, 14, AV_CODEC_ID_JPEG2000, NULL, 14, J2KWrap }, > { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x10,0x60,0x01 }, 14, AV_CODEC_ID_H264, NULL, 15 }, /* H.264 */ > { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 }, 14, AV_CODEC_ID_DNXHD, NULL, 14 }, /* VC-3 */ > + { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1e,0x01,0x00 }, 14, AV_CODEC_ID_DNXUC, NULL, 14 }, /* DNxUncompressed / SMPTE RDD 50 */ Here we have 14.. Also maybe we shouldn't copypaste ULs like this? /Tomas
On 10.09.24 15:14, Tomas Härdin wrote: > tis 2024-09-10 klockan 16:06 +0200 skrev Martin Schitter: >> --- >> libavformat/mxf.c | 1 + >> libavformat/mxfdec.c | 1 + >> 2 files changed, 2 insertions(+) > > Commit message could be better, something like "Add DNxUncompressed > ULs" O.k. -- this will be "libavformat/mxf: add DNxUncompressed MXF ULs" in the corrected patch set. >> +++ b/libavformat/mxf.c >> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x1E,0x01,0x00 }, 16, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ > > Are really all 16 bytes significant? > >> +++ b/libavformat/mxfdec.c >> + { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1e,0x01,0x00 }, 14, AV_CODEC_ID_DNXUC, NULL, 14 }, /* DNxUncompressed / SMPTE RDD 50 */ > > Here we have 14.. Also maybe we shouldn't copypaste ULs like this? Thanks for finding this obvious discrepancy. Indeed, 14 bytes should be searched (15th signifies frame/clip wrapped and 16th is just reserved). In real life I've only found and tested the ...02 1E 01 00 variant until now, but clip wrapped content should hopefully work as well. Will be fixed. Martin
On 10.09.24 15:14, Tomas Härdin wrote: >> +++ b/libavformat/mxf.c >> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x1E,0x01,0x00 }, 16, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ > > Are really all 16 bytes significant? I have to correct myself again: This Entry in the "PictureEssenceCoding" list of 'mxf.c' should according to the specification look like: + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x07,0x01,0x00 }, 15, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ The significant bit count is set to 15, because we do not support any of the extended fix point format variants. But in practice I couldn't find such an entry in any of my test samples generated by DaVinci Resolve until now. btw. there is an annoying flaw in the ffmpeg mxf parser: The very common 'fill' boxes, which are used in DNxUncompressed files to align 'pack' boxes on 265 byte boundaries, are not recognized by ffmpegs mxf parser and generate lots of "Dark key 06.0e.2b.34.01.01.01.02.03.01.02.10.01.00.00.00" warnings. Martin
ons 2024-09-11 klockan 01:08 +0200 skrev martin schitter: > > > On 10.09.24 15:14, Tomas Härdin wrote: > > > +++ b/libavformat/mxf.c > > > > + { { > > > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02, > > > 0x1E,0x01,0x00 }, 16, AV_CODEC_ID_DNXUC }, /* > > > DNxUncompressed/SMPTE RDD 50 */ > > > > Are really all 16 bytes significant? > > I have to correct myself again: > > This Entry in the "PictureEssenceCoding" list of 'mxf.c' should > according to the specification look like: > > + { { > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x07 > ,0x01,0x00 > }, 15, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ > > The significant bit count is set to 15, because we do not support any > of > the extended fix point format variants. Ah, didn't know. But this happens often with ULs. One must carefully read the wrapping spec! > btw. there is an annoying flaw in the ffmpeg mxf parser: > > The very common 'fill' boxes, which are used in DNxUncompressed files > to > align 'pack' boxes on 265 byte boundaries, are not recognized by > ffmpegs > mxf parser and generate lots of > "Dark key 06.0e.2b.34.01.01.01.02.03.01.02.10.01.00.00.00" warnings. Patch welcome ;) But surely we already deal with KAG fill? Maybe the UL for it just has too large a matching length.. But looking at mxf_metadata_read_table[] and searching for "fill" it appears we don't? /Tomas
On Wed, 11 Sep 2024, Tomas Härdin wrote: > ons 2024-09-11 klockan 01:08 +0200 skrev martin schitter: >> >> >> On 10.09.24 15:14, Tomas Härdin wrote: >> > > +++ b/libavformat/mxf.c >> >> > > + { { >> > > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02, >> > > 0x1E,0x01,0x00 }, 16, AV_CODEC_ID_DNXUC }, /* >> > > DNxUncompressed/SMPTE RDD 50 */ >> > >> > Are really all 16 bytes significant? >> >> I have to correct myself again: >> >> This Entry in the "PictureEssenceCoding" list of 'mxf.c' should >> according to the specification look like: >> >> + { { >> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x07 >> ,0x01,0x00 >> }, 15, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ >> >> The significant bit count is set to 15, because we do not support any >> of >> the extended fix point format variants. > > Ah, didn't know. But this happens often with ULs. One must carefully > read the wrapping spec! Actually it should match 14 bytes, because byte 14 defines the codec being DNXUC. Byte 15 defines the "profile" of the codec (standard format, or extended fixed point formats), but from the demuxer's point of view it is irrelevant what profiles / codec features ffmpeg's own decoder supports. E.g. the user might use the demuxer only and decode the essence on its own. Regards, Marton > > >> btw. there is an annoying flaw in the ffmpeg mxf parser: >> >> The very common 'fill' boxes, which are used in DNxUncompressed files >> to >> align 'pack' boxes on 265 byte boundaries, are not recognized by >> ffmpegs >> mxf parser and generate lots of >> "Dark key 06.0e.2b.34.01.01.01.02.03.01.02.10.01.00.00.00" warnings. > > Patch welcome ;) But surely we already deal with KAG fill? Maybe the UL > for it just has too large a matching length.. But looking at > mxf_metadata_read_table[] and searching for "fill" it appears we don't? > > /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".
On 11.09.24 21:41, Marton Balint wrote: >>> I have to correct myself again: >>> >>> This Entry in the "PictureEssenceCoding" list of 'mxf.c' should >>> according to the specification look like: >>> >>> + >>> { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x07 >>> ,0x01,0x00 }, 15, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE >>> RDD 50 */ >>> >>> The significant bit count is set to 15, because we do not support any >>> of the extended fix point format variants. >> >> Ah, didn't know. But this happens often with ULs. One must carefully >> read the wrapping spec! > > Actually it should match 14 bytes, because byte 14 defines the codec > being DNXUC. Byte 15 defines the "profile" of the codec (standard > format, or extended fixed point formats), but from the demuxer's point > of view it is irrelevant what profiles / codec features ffmpeg's own > decoder supports. E.g. the user might use the demuxer only and decode > the essence on its own. Yes -- that's indeed a good point! It also makes the error report for unsupported variants more expressive, when we only later deny processing in the decoders main dispatcher based on their unique fourcc entry. I was just irritated by this statement in the standard: "A Picture Coding Variant code of 02h shall not be used with any standard float 10- or 12-bit component values. All other fourcc codes are permitted in either variant." But this advice will only be of importance during muxing and encoding. Demux and decoding implementations can silently ignore these subtleties. Martin
diff --git a/libavformat/mxf.c b/libavformat/mxf.c index a73e40e..35fb73e 100644 --- a/libavformat/mxf.c +++ b/libavformat/mxf.c @@ -61,6 +61,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = { { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x71,0x00,0x00,0x00 }, 13, AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */ { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x03,0x02,0x00,0x00 }, 14, AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */ { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x04,0x01,0x00 }, 16, AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD Legacy Avid Media Composer MXF */ + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x1E,0x01,0x00 }, 16, AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */ { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x32,0x00,0x00 }, 14, AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC Intra */ { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x31,0x11,0x01 }, 14, AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC SPS/PPS in-band */ { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02,0x02,0x01 }, 16, AV_CODEC_ID_V210 }, /* V210 */ diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index ac63c0d..142b3e6 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -1597,6 +1597,7 @@ static const MXFCodecUL mxf_picture_essence_container_uls[] = { { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00 }, 14, AV_CODEC_ID_JPEG2000, NULL, 14, J2KWrap }, { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x10,0x60,0x01 }, 14, AV_CODEC_ID_H264, NULL, 15 }, /* H.264 */ { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 }, 14, AV_CODEC_ID_DNXHD, NULL, 14 }, /* VC-3 */ + { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1e,0x01,0x00 }, 14, AV_CODEC_ID_DNXUC, NULL, 14 }, /* DNxUncompressed / SMPTE RDD 50 */ { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x12,0x01,0x00 }, 14, AV_CODEC_ID_VC1, NULL, 14 }, /* VC-1 */ { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x14,0x01,0x00 }, 14, AV_CODEC_ID_TIFF, NULL, 14 }, /* TIFF */ { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x15,0x01,0x00 }, 14, AV_CODEC_ID_DIRAC, NULL, 14 }, /* VC-2 */