diff mbox series

[FFmpeg-devel] libopenjpeg decoder not correctly setting the pixel format for cinema JP2K wrapped MXF

Message ID CAH6sF9_T2T5Em4MRrQYk=S+7QWvMPAWnSSYnxEf4mcFeB3rFLw@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] libopenjpeg decoder not correctly setting the pixel format for cinema JP2K wrapped MXF | expand

Checks

Context Check Description
andriy/default pending
andriy/configure warning Failed to apply patch

Commit Message

Rémi Achard Aug. 30, 2020, 5:22 p.m. UTC 
Hi,

As you are probably aware, the libopenjpeg decoder is not able to interpret
cinema jp2k mxf correctly, the pixel format being assigned as rgb48 instead
of xyz12 as it should. Note that ffmpeg native jp2k decoder parse the RSIZ
descriptor from the jp2k bitstream and is able to correctly assign the
pixel format, but libopenjpeg currently don't read the RSIZ marker so that
ffmpeg wrapper has no simple mean of doing this at the moment (a pull
request on libopenjpeg is still in progress to try to fix that).

Someone recently pointed me to this commit from 2015
https://github.com/FFmpeg/FFmpeg/commit/2af260e3a85ef2a9fadcac4f4fa652cee53e591e
.
According to my tests, it doesn't work because of a wrong RSIZ key and
missing JP2KEssenceSubDescriptor lookup group. I made a patch I made to fix
that, mxfdec is now able to correctly assign the descriptor->pix_fmt field.
Note that these keys can be found in SMPTE ST 429-4 or discovered through
asdcplib with asdcp-info command line tool parsing a cinema mxf video track.

Then, even with that patch, the pixel format detected by the demuxer is not
communicated to the decoder, as far as I'm aware. Which makes me wonder,
what was the original goal of that commit and what I'm missing to
communicate this information to libopenjpeg decoder ?

 static const uint8_t mxf_avid_project_name[]               = {
0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
};
-static const uint8_t mxf_jp2k_rsiz[]                       = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
};
+static const uint8_t mxf_jp2k_rsiz[]                       = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
};
 static const uint8_t mxf_indirect_value_utf16le[]          = {
0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
};
 static const uint8_t mxf_indirect_value_utf16be[]          = {
0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
};

@@ -2746,6 +2746,7 @@ static const MXFMetadataReadTableEntry
mxf_metadata_read_table[] = {
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x42,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
Generic Sound */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x28,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
CDCI */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x29,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
RGBA */
+    { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
JPEG2000PictureSubDescriptor */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x48,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
Wave */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x47,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
AES3 */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x51,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
MPEG2VideoDescriptor */

Comments

Carl Eugen Hoyos Aug. 30, 2020, 7:31 p.m. UTC | #1 
Am So., 30. Aug. 2020 um 19:28 Uhr schrieb Rémi Achard <remiachard@gmail.com>:

> As you are probably aware, the libopenjpeg decoder is not able
> to interpret cinema jp2k mxf correctly, the pixel format being
> assigned as rgb48 instead of xyz12 as it should.

Do you have a sample that does not work with the native decoder?

Carl Eugen
Paul B Mahol Aug. 30, 2020, 7:38 p.m. UTC | #2 
On 8/30/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> Am So., 30. Aug. 2020 um 19:28 Uhr schrieb Rémi Achard
> <remiachard@gmail.com>:
>
>> As you are probably aware, the libopenjpeg decoder is not able
>> to interpret cinema jp2k mxf correctly, the pixel format being
>> assigned as rgb48 instead of xyz12 as it should.
>
> Do you have a sample that does not work with the native decoder?

I believe there are many, maybe could be generated with libopenjpeg
encoder and custom options?
Carl Eugen Hoyos Aug. 30, 2020, 8:08 p.m. UTC | #3 
Am So., 30. Aug. 2020 um 19:28 Uhr schrieb Rémi Achard <remiachard@gmail.com>:

> As you are probably aware, the libopenjpeg decoder is not able
> to interpret cinema jp2k mxf correctly, the pixel format being
> assigned as rgb48 instead of xyz12 as it should.

Do you have a cinema xyz12 sample that does not work with
the native decoder?

Carl Eugen
Carl Eugen Hoyos Aug. 30, 2020, 8:09 p.m. UTC | #4 
Am So., 30. Aug. 2020 um 21:39 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>
> On 8/30/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > Am So., 30. Aug. 2020 um 19:28 Uhr schrieb Rémi Achard
> > <remiachard@gmail.com>:
> >
> >> As you are probably aware, the libopenjpeg decoder is not able
> >> to interpret cinema jp2k mxf correctly, the pixel format being
> >> assigned as rgb48 instead of xyz12 as it should.
> >
> > Do you have a sample that does not work with the native decoder?
>
> I believe there are many, maybe could be generated with libopenjpeg
> encoder and custom options?

Not anymore, but I meant Cinema samples as in the original question.

Carl Eugen
Tomas Härdin Aug. 31, 2020, 2:26 p.m. UTC | #5 
sön 2020-08-30 klockan 18:22 +0100 skrev Rémi Achard:
> Hi,
> 
> As you are probably aware, the libopenjpeg decoder is not able to interpret
> cinema jp2k mxf correctly, the pixel format being assigned as rgb48 instead
> of xyz12 as it should. Note that ffmpeg native jp2k decoder parse the RSIZ
> descriptor from the jp2k bitstream and is able to correctly assign the
> pixel format, but libopenjpeg currently don't read the RSIZ marker so that
> ffmpeg wrapper has no simple mean of doing this at the moment (a pull
> request on libopenjpeg is still in progress to try to fix that).
> 
> Someone recently pointed me to this commit from 2015
> https://github.com/FFmpeg/FFmpeg/commit/2af260e3a85ef2a9fadcac4f4fa652cee53e591e
> .
> According to my tests, it doesn't work because of a wrong RSIZ key and
> missing JP2KEssenceSubDescriptor lookup group. I made a patch I made to fix
> that, mxfdec is now able to correctly assign the descriptor->pix_fmt field.
> Note that these keys can be found in SMPTE ST 429-4 or discovered through
> asdcplib with asdcp-info command line tool parsing a cinema mxf video track.
> 
> Then, even with that patch, the pixel format detected by the demuxer is not
> communicated to the decoder, as far as I'm aware. Which makes me wonder,
> what was the original goal of that commit and what I'm missing to
> communicate this information to libopenjpeg decoder ?
> 

Can't say much about the J2K part of this, so I'll comment on the MXF
side of things.

> -static const uint8_t mxf_jp2k_rsiz[]                       = {
> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
> };
> +static const uint8_t mxf_jp2k_rsiz[]                       = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
> };

If you want the IS_KLV_KEY() check to be more lenient then you should
make mxf_jp2k_rsiz[] shorter, not replace the end of it with NULs

> +    { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> JPEG2000PictureSubDescriptor */

Matches S422M, so should be OK.

We need a sample here, as Carl says.

/Tomas
Rémi Achard Sept. 1, 2020, 10:06 a.m. UTC | #6 
> Do you have a sample that does not work with the native decoder?

According to my tests, the native decoder detect pixel format just fine,
I'm talking about the libopenjpeg wrapper here (ie. libopenjpegdec.c).

As for sample materials, I guess you can check this link
https://github.com/Ymagis/ClairMeta_Data

Specifically, we made DCP in various configurations with MXF video track
that last 1sec (minimum allowed by the SMPTE), let me know if that works
for you.
* 2D Interop MXF
https://github.com/Ymagis/ClairMeta_Data/blob/master/DCP/ECL-SET/ECL01-SINGLE-CPL_TST_S_EN-XX_UK-U_71_2K_DI_20171218_ECL_IOP_OV/ECL01-SINGLE-CPL_TST_S_EN-XX_UK-U_71_2K_DI_20171218_ECL_IOP_OV_01.mxf
* 3D SMPTE MXF
https://github.com/Ymagis/ClairMeta_Data/blob/master/DCP/ECL-SET/ECL09-SINGLE-CPL_TST_S_EN-XX_UK-U_51-ATMOS_2K_DI_20171220_ECL_SMPTE_OV/ECL-SINGLE-CPL_TST_S_EN-XX_UK-U_51_2K_DI_20171220_ECL_SMPTE_OV_01.mxf

> If you want the IS_KLV_KEY() check to be more lenient then you should
> make mxf_jp2k_rsiz[] shorter, not replace the end of it with NULs

This UL come straight from SMPTE ST 422M, not sure what you mean but I
didn't replace it with arbitrary NULs values.


Looking at mxfdec.c some more, I found that the pixel format is
supposed to be passed to the AVStream codecpar later on, but there is
some issues with that, that I don't found a solution yet:

* The JPEG2000PictureSubDescriptor live in it's own "MetadatSet" and
the pixel_format assigned is from a different Descriptor structure
than the one used for eg. width and height parameters
* Currently the pixel_format is only passed on for RAWVIDEO codec, so
it would require doing that for JP2K and potentially others as well


Le dim. 30 août 2020 à 18:22, Rémi Achard <remiachard@gmail.com> a écrit :

> Hi,
>
> As you are probably aware, the libopenjpeg decoder is not able to
> interpret cinema jp2k mxf correctly, the pixel format being assigned as
> rgb48 instead of xyz12 as it should. Note that ffmpeg native jp2k decoder
> parse the RSIZ descriptor from the jp2k bitstream and is able to correctly
> assign the pixel format, but libopenjpeg currently don't read the
> RSIZ marker so that ffmpeg wrapper has no simple mean of doing this at the
> moment (a pull request on libopenjpeg is still in progress to try to fix
> that).
>
> Someone recently pointed me to this commit from 2015
> https://github.com/FFmpeg/FFmpeg/commit/2af260e3a85ef2a9fadcac4f4fa652cee53e591e .
> According to my tests, it doesn't work because of a wrong RSIZ key and
> missing JP2KEssenceSubDescriptor lookup group. I made a patch I made to fix
> that, mxfdec is now able to correctly assign the descriptor->pix_fmt field.
> Note that these keys can be found in SMPTE ST 429-4 or discovered through
> asdcplib with asdcp-info command line tool parsing a cinema mxf video track.
>
> Then, even with that patch, the pixel format detected by the demuxer is
> not communicated to the decoder, as far as I'm aware. Which makes me
> wonder, what was the original goal of that commit and what I'm missing to
> communicate this information to libopenjpeg decoder ?
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 6f6e8d586c..cd85fbedff 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -325,7 +325,7 @@ static const uint8_t mxf_encrypted_essence_container[]
>     = { 0x06,0x0e,0x2b,0x
>  static const uint8_t mxf_random_index_pack_key[]           = {
> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
> };
>  static const uint8_t mxf_sony_mpeg4_extradata[]            = {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
> };
>  static const uint8_t mxf_avid_project_name[]               = {
> 0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
> };
> -static const uint8_t mxf_jp2k_rsiz[]                       = {
> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
> };
> +static const uint8_t mxf_jp2k_rsiz[]                       = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
> };
>  static const uint8_t mxf_indirect_value_utf16le[]          = {
> 0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
> };
>  static const uint8_t mxf_indirect_value_utf16be[]          = {
> 0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
> };
>
> @@ -2746,6 +2746,7 @@ static const MXFMetadataReadTableEntry
> mxf_metadata_read_table[] = {
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x42,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> Generic Sound */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x28,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> CDCI */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x29,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> RGBA */
> +    { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> JPEG2000PictureSubDescriptor */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x48,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> Wave */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x47,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> AES3 */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x51,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> MPEG2VideoDescriptor */
>
>
Rémi Achard Sept. 1, 2020, 10:26 a.m. UTC | #7 
> * 3D SMPTE MXF

Typo, this is 2D SMPTE MXF, there are 3D versions in that same repo
however, but it probably don't matter much here.

Le mar. 1 sept. 2020 à 11:06, Rémi Achard <remiachard@gmail.com> a écrit :

> > Do you have a sample that does not work with the native decoder?
>
> According to my tests, the native decoder detect pixel format just fine,
> I'm talking about the libopenjpeg wrapper here (ie. libopenjpegdec.c).
>
> As for sample materials, I guess you can check this link
> https://github.com/Ymagis/ClairMeta_Data
>
> Specifically, we made DCP in various configurations with MXF video track
> that last 1sec (minimum allowed by the SMPTE), let me know if that works
> for you.
> * 2D Interop MXF
> https://github.com/Ymagis/ClairMeta_Data/blob/master/DCP/ECL-SET/ECL01-SINGLE-CPL_TST_S_EN-XX_UK-U_71_2K_DI_20171218_ECL_IOP_OV/ECL01-SINGLE-CPL_TST_S_EN-XX_UK-U_71_2K_DI_20171218_ECL_IOP_OV_01.mxf
> * 3D SMPTE MXF
> https://github.com/Ymagis/ClairMeta_Data/blob/master/DCP/ECL-SET/ECL09-SINGLE-CPL_TST_S_EN-XX_UK-U_51-ATMOS_2K_DI_20171220_ECL_SMPTE_OV/ECL-SINGLE-CPL_TST_S_EN-XX_UK-U_51_2K_DI_20171220_ECL_SMPTE_OV_01.mxf
>
> > If you want the IS_KLV_KEY() check to be more lenient then you should
> > make mxf_jp2k_rsiz[] shorter, not replace the end of it with NULs
>
> This UL come straight from SMPTE ST 422M, not sure what you mean but I didn't replace it with arbitrary NULs values.
>
>
> Looking at mxfdec.c some more, I found that the pixel format is supposed to be passed to the AVStream codecpar later on, but there is some issues with that, that I don't found a solution yet:
>
> * The JPEG2000PictureSubDescriptor live in it's own "MetadatSet" and the pixel_format assigned is from a different Descriptor structure than the one used for eg. width and height parameters
> * Currently the pixel_format is only passed on for RAWVIDEO codec, so it would require doing that for JP2K and potentially others as well
>
>
> Le dim. 30 août 2020 à 18:22, Rémi Achard <remiachard@gmail.com> a écrit :
>
>> Hi,
>>
>> As you are probably aware, the libopenjpeg decoder is not able to
>> interpret cinema jp2k mxf correctly, the pixel format being assigned as
>> rgb48 instead of xyz12 as it should. Note that ffmpeg native jp2k decoder
>> parse the RSIZ descriptor from the jp2k bitstream and is able to correctly
>> assign the pixel format, but libopenjpeg currently don't read the
>> RSIZ marker so that ffmpeg wrapper has no simple mean of doing this at the
>> moment (a pull request on libopenjpeg is still in progress to try to fix
>> that).
>>
>> Someone recently pointed me to this commit from 2015
>> https://github.com/FFmpeg/FFmpeg/commit/2af260e3a85ef2a9fadcac4f4fa652cee53e591e .
>> According to my tests, it doesn't work because of a wrong RSIZ key and
>> missing JP2KEssenceSubDescriptor lookup group. I made a patch I made to fix
>> that, mxfdec is now able to correctly assign the descriptor->pix_fmt field.
>> Note that these keys can be found in SMPTE ST 429-4 or discovered through
>> asdcplib with asdcp-info command line tool parsing a cinema mxf video track.
>>
>> Then, even with that patch, the pixel format detected by the demuxer is
>> not communicated to the decoder, as far as I'm aware. Which makes me
>> wonder, what was the original goal of that commit and what I'm missing to
>> communicate this information to libopenjpeg decoder ?
>>
>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> index 6f6e8d586c..cd85fbedff 100644
>> --- a/libavformat/mxfdec.c
>> +++ b/libavformat/mxfdec.c
>> @@ -325,7 +325,7 @@ static const uint8_t
>> mxf_encrypted_essence_container[]     = { 0x06,0x0e,0x2b,0x
>>  static const uint8_t mxf_random_index_pack_key[]           = {
>> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
>> };
>>  static const uint8_t mxf_sony_mpeg4_extradata[]            = {
>> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
>> };
>>  static const uint8_t mxf_avid_project_name[]               = {
>> 0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
>> };
>> -static const uint8_t mxf_jp2k_rsiz[]                       = {
>> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
>> };
>> +static const uint8_t mxf_jp2k_rsiz[]                       = {
>> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
>> };
>>  static const uint8_t mxf_indirect_value_utf16le[]          = {
>> 0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
>> };
>>  static const uint8_t mxf_indirect_value_utf16be[]          = {
>> 0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
>> };
>>
>> @@ -2746,6 +2746,7 @@ static const MXFMetadataReadTableEntry
>> mxf_metadata_read_table[] = {
>>      { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x42,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> Generic Sound */
>>      { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x28,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> CDCI */
>>      { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x29,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> RGBA */
>> +    { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> JPEG2000PictureSubDescriptor */
>>      { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x48,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> Wave */
>>      { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x47,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> AES3 */
>>      { {
>> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x51,0x00
>> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
>> MPEG2VideoDescriptor */
>>
>>
Carl Eugen Hoyos Sept. 1, 2020, 5:17 p.m. UTC | #8 
Am Di., 1. Sept. 2020 um 12:14 Uhr schrieb Rémi Achard <remiachard@gmail.com>:
>
> > Do you have a sample that does not work with the native decoder?
>
> According to my tests, the native decoder detect pixel format just fine

So what problem do you see?
Yes, we prefer using the native codecs over external dependencies.

You can force the pix_fmt, Carl Eugen
Rémi Achard Sept. 1, 2020, 5:32 p.m. UTC | #9 
> So what problem do you see?
> Yes, we prefer using the native codecs over external dependencies.

ffmpeg -v debug -vcodec libopenjpeg -i
~/Projects/ClairMeta/tests/resources/DCP/ECL-SET/ECL07-SINGLE-CPL_TST-3D-48_S_EN-XX_UK-U_71-ATMOS_2K_ECL_20180301_ECL_SMPTE-3D_OV/ECL07SingleCPL_TST-3D-48_S_EN-XX_UK-U_71-ATMOS_2K_ECL_20180301_ECL_SMPTE-3D_OV_01.mxf
...
Stream #0:0, 1, 1/48: Video: jpeg2000, 1 reference frame, rgb48le(12 bpc,
progressive), 2048x858, 0/1, SAR 1:1 DAR 1024:429, 48 tbr, 48 tbn, 48 tbc
...

rgb48le is not correct as this file is xyz12

We can use the native decoder yes, not sure what the state is right now
though, because I heard that some work was ongoing on that decoder, but
back then (approx. 1 year ago) it was not really an option due to its poor
performance, even for offline jobs.

> You can force the pix_fmt, Carl Eugen

How can I do that ?

I'll continue to work on trying to fix mxfdec.c anyway, thanks.

Le mar. 1 sept. 2020 à 18:18, Carl Eugen Hoyos <ceffmpeg@gmail.com> a
écrit :

> Am Di., 1. Sept. 2020 um 12:14 Uhr schrieb Rémi Achard <
> remiachard@gmail.com>:
> >
> > > Do you have a sample that does not work with the native decoder?
> >
> > According to my tests, the native decoder detect pixel format just fine
>
> So what problem do you see?
> Yes, we prefer using the native codecs over external dependencies.
>
> You can force the pix_fmt, Carl Eugen
> _______________________________________________
> 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".
Carl Eugen Hoyos Sept. 1, 2020, 5:39 p.m. UTC | #10 
Am Di., 1. Sept. 2020 um 19:32 Uhr schrieb Rémi Achard <remiachard@gmail.com>:

> We can use the native decoder yes, not sure what the state is right now
> though, because I heard that some work was ongoing on that decoder, but
> back then (approx. 1 year ago) it was not really an option due to its poor
> performance, even for offline jobs.

Please elaborate!

Carl Eugen
Rémi Achard Sept. 1, 2020, 5:57 p.m. UTC | #11 
> Please elaborate!

Sure, here are some comparisons I just made, please note the huge
difference in speed between current master and n4.0 that I picked as a
representative of the state of the decoder back then.

There are great improvements being made for sure, I still think there could
be some benefit having the libopenjpeg decoder work out of the box for
cinema j2k (one of the big user of that format probably).

libopenjpeg (master)
frame=   96 fps= 14 q=-0.0 Lsize=   59514kB time=00:00:01.97
bitrate=246323.9kbits/s speed=0.279x
jpeg2000 (master)
frame=   96 fps=8.8 q=-0.0 Lsize=   70102kB time=00:00:01.97
bitrate=290149.4kbits/s speed=0.182x
jpeg2000 (n4.0)
frame=   50 fps=3.0 q=-0.0 Lsize=   36176kB time=00:00:01.02
bitrate=290281.1kbits/s dup=0 drop=46 speed=0.0605x

Le mar. 1 sept. 2020 à 18:39, Carl Eugen Hoyos <ceffmpeg@gmail.com> a
écrit :

> Am Di., 1. Sept. 2020 um 19:32 Uhr schrieb Rémi Achard <
> remiachard@gmail.com>:
>
> > We can use the native decoder yes, not sure what the state is right now
> > though, because I heard that some work was ongoing on that decoder, but
> > back then (approx. 1 year ago) it was not really an option due to its
> poor
> > performance, even for offline jobs.
>
> Please elaborate!
>
> Carl Eugen
> _______________________________________________
> 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".
Rémi Achard Sept. 1, 2020, 5:59 p.m. UTC | #12 
> Sure, here are some comparisons I just made, please note the huge
difference in speed between current master and n4.0 that I picked as a
representative of the state of the decoder back then.

Also worth noting that the n4.0 decoder was not even able to transcode the
full sequence properly and output a huge amount of warnings / errors.

>
Rémi Achard Sept. 1, 2020, 10:43 p.m. UTC | #13 
> I'll continue to work on trying to fix mxfdec.c anyway, thanks.

Update to the patch proposal, seems to be working ok (as far as pixel
format is concerned) for both SMPTE & INTEROP DCP.


diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 6f6e8d586c..4c146a152d 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -325,9 +325,11 @@ static const uint8_t mxf_encrypted_essence_container[]
    = { 0x06,0x0e,0x2b,0x
 static const uint8_t mxf_random_index_pack_key[]           = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
};
 static const uint8_t mxf_sony_mpeg4_extradata[]            = {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
};
 static const uint8_t mxf_avid_project_name[]               = {
0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
};
-static const uint8_t mxf_jp2k_rsiz[]                       = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
};
+static const uint8_t mxf_jp2k_rsiz[]                       = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
};
 static const uint8_t mxf_indirect_value_utf16le[]          = {
0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
};
 static const uint8_t mxf_indirect_value_utf16be[]          = {
0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
};
+static const uint8_t mxf_subdescriptor_array_smpte[]       = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00
};
+static const uint8_t mxf_subdescriptor_array_interop[]     = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00
};

 #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))

@@ -1272,6 +1274,11 @@ static int mxf_read_generic_descriptor(void *arg,
AVIOContext *pb, int tag, int
                 rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K)
                 descriptor->pix_fmt = AV_PIX_FMT_XYZ12;
         }
+        if (IS_KLV_KEY(uid, mxf_subdescriptor_array_smpte)
+         || IS_KLV_KEY(uid, mxf_subdescriptor_array_interop)) {
+            mxf_read_strong_ref_array(pb,
&descriptor->sub_descriptors_refs,
+
 &descriptor->sub_descriptors_count);
+        }
         break;
     }
     return 0;
@@ -2498,6 +2505,16 @@ static int mxf_parse_structural_metadata(MXFContext
*mxf)
                 }
             }

+            if (st->codecpar->codec_id == AV_CODEC_ID_JPEG2000) {
+                MXFDescriptor *desc = NULL;
+                for (k = 0; k < descriptor->sub_descriptors_count; k++) {
+                    if ((desc = mxf_resolve_strong_ref(mxf,
&descriptor->sub_descriptors_refs[k], SubDescriptor))) {
+                        st->codecpar->format = desc->pix_fmt;
+                        break;
+                    }
+                }
+            }
+
             if (st->codecpar->codec_id == AV_CODEC_ID_RAWVIDEO) {
                 st->codecpar->format = descriptor->pix_fmt;
                 if (st->codecpar->format == AV_PIX_FMT_NONE) {
@@ -2753,6 +2770,7 @@ static const MXFMetadataReadTableEntry
mxf_metadata_read_table[] = {
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5c,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
VANC/VBI - SMPTE 436M */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5e,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
MPEG2AudioDescriptor */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x64,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* DC
Timed Text Descriptor */
+    { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), SubDescriptor }, /*
JPEG2000PictureSubDescriptor */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00
}, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00
}, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00
}, mxf_read_timecode_component, sizeof(MXFTimecodeComponent),
TimecodeComponent },

Le mar. 1 sept. 2020 à 18:59, Rémi Achard <remiachard@gmail.com> a écrit :

> > Sure, here are some comparisons I just made, please note the huge
> difference in speed between current master and n4.0 that I picked as a
> representative of the state of the decoder back then.
>
> Also worth noting that the n4.0 decoder was not even able to transcode the
> full sequence properly and output a huge amount of warnings / errors.
>
>>
Rémi Achard Sept. 2, 2020, 7:02 p.m. UTC | #14 
Small update to the previous patch, adding a filter on
JPEG2000PictureSubDescriptor when iterating through the list of
SubDescriptors.

diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index fc587f19f0..3fb3c6d74d 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -49,6 +49,7 @@ enum MXFMetadataSetType {
     TaggedValue,
     TapeDescriptor,
     AVCSubDescriptor,
+    JPEG2000PictureSubDescriptor,
 };

 enum MXFFrameLayout {
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 6f6e8d586c..8eac7fc944 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -325,9 +325,11 @@ static const uint8_t mxf_encrypted_essence_container[]
    = { 0x06,0x0e,0x2b,0x
 static const uint8_t mxf_random_index_pack_key[]           = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
};
 static const uint8_t mxf_sony_mpeg4_extradata[]            = {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
};
 static const uint8_t mxf_avid_project_name[]               = {
0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
};
-static const uint8_t mxf_jp2k_rsiz[]                       = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
};
+static const uint8_t mxf_jp2k_rsiz[]                       = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
};
 static const uint8_t mxf_indirect_value_utf16le[]          = {
0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
};
 static const uint8_t mxf_indirect_value_utf16be[]          = {
0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
};
+static const uint8_t mxf_subdescriptor_array_smpte[]       = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00
};
+static const uint8_t mxf_subdescriptor_array_interop[]     = {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00
};

 #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))

@@ -1272,6 +1274,11 @@ static int mxf_read_generic_descriptor(void *arg,
AVIOContext *pb, int tag, int
                 rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K)
                 descriptor->pix_fmt = AV_PIX_FMT_XYZ12;
         }
+        if (IS_KLV_KEY(uid, mxf_subdescriptor_array_smpte)
+         || IS_KLV_KEY(uid, mxf_subdescriptor_array_interop)) {
+            mxf_read_strong_ref_array(pb,
&descriptor->sub_descriptors_refs,
+
 &descriptor->sub_descriptors_count);
+        }
         break;
     }
     return 0;
@@ -2498,6 +2505,16 @@ static int mxf_parse_structural_metadata(MXFContext
*mxf)
                 }
             }

+            if (st->codecpar->codec_id == AV_CODEC_ID_JPEG2000) {
+                MXFDescriptor *desc = NULL;
+                for (k = 0; k < descriptor->sub_descriptors_count; k++) {
+                    if ((desc = mxf_resolve_strong_ref(mxf,
&descriptor->sub_descriptors_refs[k], JPEG2000PictureSubDescriptor))) {
+                        st->codecpar->format = desc->pix_fmt;
+                        break;
+                    }
+                }
+            }
+
             if (st->codecpar->codec_id == AV_CODEC_ID_RAWVIDEO) {
                 st->codecpar->format = descriptor->pix_fmt;
                 if (st->codecpar->format == AV_PIX_FMT_NONE) {
@@ -2753,6 +2770,7 @@ static const MXFMetadataReadTableEntry
mxf_metadata_read_table[] = {
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5c,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
VANC/VBI - SMPTE 436M */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5e,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
MPEG2AudioDescriptor */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x64,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* DC
Timed Text Descriptor */
+    { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
}, mxf_read_generic_descriptor, sizeof(MXFDescriptor),
JPEG2000PictureSubDescriptor }, /* JPEG2000PictureSubDescriptor */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00
}, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00
}, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
     { {
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00
}, mxf_read_timecode_component, sizeof(MXFTimecodeComponent),
TimecodeComponent },

Le mar. 1 sept. 2020 à 23:43, Rémi Achard <remiachard@gmail.com> a écrit :

> > I'll continue to work on trying to fix mxfdec.c anyway, thanks.
>
> Update to the patch proposal, seems to be working ok (as far as pixel
> format is concerned) for both SMPTE & INTEROP DCP.
>
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 6f6e8d586c..4c146a152d 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -325,9 +325,11 @@ static const uint8_t
> mxf_encrypted_essence_container[]     = { 0x06,0x0e,0x2b,0x
>  static const uint8_t mxf_random_index_pack_key[]           = {
> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
> };
>  static const uint8_t mxf_sony_mpeg4_extradata[]            = {
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
> };
>  static const uint8_t mxf_avid_project_name[]               = {
> 0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
> };
> -static const uint8_t mxf_jp2k_rsiz[]                       = {
> 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
> };
> +static const uint8_t mxf_jp2k_rsiz[]                       = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00
> };
>  static const uint8_t mxf_indirect_value_utf16le[]          = {
> 0x4c,0x00,0x02,0x10,0x01,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
> };
>  static const uint8_t mxf_indirect_value_utf16be[]          = {
> 0x42,0x01,0x10,0x02,0x00,0x00,0x00,0x00,0x00,0x06,0x0e,0x2b,0x34,0x01,0x04,0x01,0x01
> };
> +static const uint8_t mxf_subdescriptor_array_smpte[]       = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00
> };
> +static const uint8_t mxf_subdescriptor_array_interop[]     = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00
> };
>
>  #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))
>
> @@ -1272,6 +1274,11 @@ static int mxf_read_generic_descriptor(void *arg,
> AVIOContext *pb, int tag, int
>                  rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K)
>                  descriptor->pix_fmt = AV_PIX_FMT_XYZ12;
>          }
> +        if (IS_KLV_KEY(uid, mxf_subdescriptor_array_smpte)
> +         || IS_KLV_KEY(uid, mxf_subdescriptor_array_interop)) {
> +            mxf_read_strong_ref_array(pb,
> &descriptor->sub_descriptors_refs,
> +
>  &descriptor->sub_descriptors_count);
> +        }
>          break;
>      }
>      return 0;
> @@ -2498,6 +2505,16 @@ static int mxf_parse_structural_metadata(MXFContext
> *mxf)
>                  }
>              }
>
> +            if (st->codecpar->codec_id == AV_CODEC_ID_JPEG2000) {
> +                MXFDescriptor *desc = NULL;
> +                for (k = 0; k < descriptor->sub_descriptors_count; k++) {
> +                    if ((desc = mxf_resolve_strong_ref(mxf,
> &descriptor->sub_descriptors_refs[k], SubDescriptor))) {
> +                        st->codecpar->format = desc->pix_fmt;
> +                        break;
> +                    }
> +                }
> +            }
> +
>              if (st->codecpar->codec_id == AV_CODEC_ID_RAWVIDEO) {
>                  st->codecpar->format = descriptor->pix_fmt;
>                  if (st->codecpar->format == AV_PIX_FMT_NONE) {
> @@ -2753,6 +2770,7 @@ static const MXFMetadataReadTableEntry
> mxf_metadata_read_table[] = {
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5c,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> VANC/VBI - SMPTE 436M */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5e,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /*
> MPEG2AudioDescriptor */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x64,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* DC
> Timed Text Descriptor */
> +    { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5a,0x00
> }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), SubDescriptor }, /*
> JPEG2000PictureSubDescriptor */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00
> }, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00
> }, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
>      { {
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00
> }, mxf_read_timecode_component, sizeof(MXFTimecodeComponent),
> TimecodeComponent },
>
> Le mar. 1 sept. 2020 à 18:59, Rémi Achard <remiachard@gmail.com> a écrit :
>
>> > Sure, here are some comparisons I just made, please note the huge
>> difference in speed between current master and n4.0 that I picked as a
>> representative of the state of the decoder back then.
>>
>> Also worth noting that the n4.0 decoder was not even able to transcode
>> the full sequence properly and output a huge amount of warnings / errors.
>>
>>>
Tomas Härdin Sept. 4, 2020, 8:33 a.m. UTC | #15 
tis 2020-09-01 klockan 11:06 +0100 skrev Rémi Achard:
> > If you want the IS_KLV_KEY() check to be more lenient then you should
> > make mxf_jp2k_rsiz[] shorter, not replace the end of it with NULs
> 
> This UL come straight from SMPTE ST 422M, not sure what you mean but I
> didn't replace it with arbitrary NULs values.

You're right, it's right there on page 10 of S422M-2006. This makes me
wonder where the old value came from. I missed that many bytes of it
were different, not just that the end was nulled. There is also no
match for "0d.01" in S422M like there is for "04.01.06.03".

In short: the UL matches the spec so it is fine

/Tomas
diff mbox series

Patch

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 6f6e8d586c..cd85fbedff 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -325,7 +325,7 @@  static const uint8_t mxf_encrypted_essence_container[]
    = { 0x06,0x0e,0x2b,0x
 static const uint8_t mxf_random_index_pack_key[]           = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
};
 static const uint8_t mxf_sony_mpeg4_extradata[]            = {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
};