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 |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/configure | warning | Failed to apply patch |
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
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?
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
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
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
> 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 */ > >
> * 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 */ >> >>
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
> 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".
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
> 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".
> 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. >
> 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. > >>
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. >> >>>
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 --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 };