[FFmpeg-devel] [PATCH] avformat/mxfdec: Detect XYZ pixel format for digital cinema files
While the native jpeg2000 decoder can determine pixel format correctly
from the codestream, libopenjpeg wrapper cannot. To make sure that
the output is correct when using libopenjpeg to decode digital
cinema files, we do detection from the metadata included in the MXF
wrapper.
If the container has JPEG 2000 Coding Parameters metadata element
with Rsiz value set to one of digital cinema profiles, we can safely
assume that the given input file is DCI compliant, therefore the
pixel format should be XYZ.
---
libavformat/mxfdec.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index f3501da..2e8dd05 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -281,6 +281,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
};
#define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))
@@ -1000,6 +1001,12 @@ static int mxf_read_generic_descriptor(void *arg,
AVIOContext *pb, int tag, int
descriptor-extradata_size = size;
avio_read(pb, descriptor-extradata, size);
}
+if (IS_KLV_KEY(uid, mxf_jp2k_rsiz)) {
+uint32_t rsiz = avio_rb16(pb);
+if (rsiz == FF_PROFILE_JPEG2000_DCINEMA_2K ||
+rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K)
+descriptor-pix_fmt = AV_PIX_FMT_XYZ12;
+}
break;
}
return 0;
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 4, 2015 at 1:03 AM, Michael Niedermayer michae...@gmx.at wrote: this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? No, this seems to be a regression. At the moment there is not enough information in openjpeg data structures (opj_image_t specifically) to reliably determine whether given image is is 12-bit XYZ or 12-bit RGB. I do not know how the best way to handle this case. Unless we can find some additional way to determine this, one of these cases will be broken. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 4, 2015 at 10:08 AM, Vilius Grigaliūnas vilius.grigaliu...@gmail.com wrote: On Wed, Mar 4, 2015 at 1:03 AM, Michael Niedermayer michae...@gmx.at wrote: this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? No, this seems to be a regression. At the moment there is not enough information in openjpeg data structures (opj_image_t specifically) to reliably determine whether given image is is 12-bit XYZ or 12-bit RGB. I do not know how the best way to handle this case. Unless we can find some additional way to determine this, one of these cases will be broken. AFAIK there is no way to reliably determine color space just from naked J2K codestream. It basically boils down to a guess. In my opinion assuming that the value is 12-bit XYZ is just as valid as 12-bit RBG. I think in cases like this there should be a way to a pass pixel format value as input option. The way I understand jpeg2000 standard, XYZ interpretation would be more correct unless JP2 header or other source indicates otherwise. But I see how this would lead to mostly unexpected behavior in a lot of real-world cases. On Wed, Mar 4, 2015 at 12:10 PM, Michael Niedermayer michae...@gmx.at wrote: the best way would be to improve our jpeg2000 decoder so its at least as good as libopenjpeg then it can be used and it would have access to everything from the bitstream to determine the colorspace ... That would be great, but that does not mean that we should leave libopenjpeg decoder broken in specific cases (as in Digital Cinema which is one of the main use cases for JPEG200). I still see the utility in having it as it will have wider format support for the foreseeable future. Native jpeg2000 only uses xyz pixels if it detects that the file in Digital Cinema profile (RSiz = 3 or RSiz = 4). While libopenjpeg does not expose this, it is also stored in MXF container which is the standard wrapper for DC j2k streams. That means we can detect xyz pixels in avformat/mxfdec and pass it to decoder. Which might be an even better solution. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 4, 2015 at 4:48 PM, Michael Niedermayer michae...@gmx.at wrote: i did not mean to suggest to leave libopenjpeg broken. It is not technically broken, it just relies on guessing in some cases which might not always be correct. It think there should be a warning that this is happening and a way to override it. I had made a patch that allows to set pix_fmt via codec options. But I found out that the codec gets called two times, first without options to detect format and then with options to perform decoding. That means that the pixel format changes after the decoding starts. Although it works I do not find it very elegant. Anyway, I will try to implement pixel format detection in MXF decoder and that should solve the issue for my use case. Just that in every case our native decoders replaced external wrapers. If this happens with libopenjpeg too, all work on the wraper will be in vain as it will not be used anymore after some point. so i was imlpicitly thinking a bit ahead like waht will be there in the long term, lets concentrate work on that rather than the short term but by all means if you like improvig the libopenjpeg wraper, do so I thought this would be a quick fix, but the reality turned out to be a bit more complicated. :) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
Input files in XYZ color space are incorrecly detected as RGB which results
in incorrect output colors.
This fixes pixel format detection order (in increasing bit depth to
match libopenjpeg_matches_pix_fmt) when color space provided by
libopenjepg is unknown.
---
libavcodec/libopenjpegdec.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
index 1cd1b9b..489040e 100644
--- a/libavcodec/libopenjpegdec.c
+++ b/libavcodec/libopenjpegdec.c
@@ -77,7 +77,7 @@ static const enum AVPixelFormat libopenjpeg_yuv_pix_fmts[] =
{
YUV_PIXEL_FORMATS
};
static const enum AVPixelFormat libopenjpeg_all_pix_fmts[] = {
-RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS, XYZ_PIXEL_FORMATS
+AV_PIX_FMT_RGB24, AV_PIX_FMT_RGBA, XYZ_PIXEL_FORMATS, AV_PIX_FMT_RGB48,
AV_PIX_FMT_RGBA64, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS
};
typedef struct LibOpenJPEGContext {
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] acvodec/lipopenjpeg: Fix pixel value shift for 12-bit pixel formats
This fixes pixel values not being properly shifted in
libopenjpeg_copyto16 and libopenjpeg_copy_to_packed16 methods.
Pixel formats like xyz12le need to be shifted by AVComponentDescriptor::shift
to get the correct values.
---
libavcodec/libopenjpegdec.c |6 [32m[m[31m--[m
1 file changed, 4 insertions(+), 2 deletions(-)
[1mdiff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c[m
[1mindex 02b1ceb..1cd1b9b 100644[m
[1m--- a/libavcodec/libopenjpegdec.c[m
[1m+++ b/libavcodec/libopenjpegdec.c[m
[36m@@ -184,10 +184,11 @@[m [mstatic inline void
libopenjpeg_copy_to_packed8(AVFrame *picture, opj_image_t *im[m
[m
static inline void libopenjpeg_copy_to_packed16(AVFrame *picture, opj_image_t
*image) {[m
uint16_t *img_ptr;[m
[32m+[m[32mconst AVPixFmtDescriptor *desc =
av_pix_fmt_desc_get(picture-format);[m
int index, x, y, c;[m
int adjust[4];[m
for (x = 0; x image-numcomps; x++)[m
[31m-adjust[x] =
FFMAX(FFMIN(av_pix_fmt_desc_get(picture-format)-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0);[m
[32m+[m[32madjust[x] = FFMAX(FFMIN(desc-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0) + desc-comp[x].shift;[m
[m
for (y = 0; y picture-height; y++) {[m
index = y * picture-width;[m
[36m@@ -220,10 +221,11 @@[m [mstatic inline void libopenjpeg_copyto8(AVFrame
*picture, opj_image_t *image) {[m
static inline void libopenjpeg_copyto16(AVFrame *picture, opj_image_t *image)
{[m
int *comp_data;[m
uint16_t *img_ptr;[m
[32m+[m[32mconst AVPixFmtDescriptor *desc =
av_pix_fmt_desc_get(picture-format);[m
int index, x, y;[m
int adjust[4];[m
for (x = 0; x image-numcomps; x++)[m
[31m-adjust[x] =
FFMAX(FFMIN(av_pix_fmt_desc_get(picture-format)-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0);[m
[32m+[m[32madjust[x] = FFMAX(FFMIN(desc-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0) + desc-comp[x].shift;[m
[m
for (index = 0; index image-numcomps; index++) {[m
comp_data = image-comps[index].data;[m
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
Input files in XYZ color space are incorrecly detected as RGB which results
in incorrect output colors.
This fixes pixel format detection order (in increasing bit depth to
match libopenjpeg_matches_pix_fmt) when color space provided by
libopenjepg is unknown.
---
libavcodec/libopenjpegdec.c |2 [32m+[m[31m-[m
1 file changed, 1 insertion(+), 1 deletion(-)
[1mdiff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c[m
[1mindex 1cd1b9b..489040e 100644[m
[1m--- a/libavcodec/libopenjpegdec.c[m
[1m+++ b/libavcodec/libopenjpegdec.c[m
[36m@@ -77,7 +77,7 @@[m [mstatic const enum AVPixelFormat
libopenjpeg_yuv_pix_fmts[] = {[m
YUV_PIXEL_FORMATS[m
};[m
static const enum AVPixelFormat libopenjpeg_all_pix_fmts[] = {[m
[31m-RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS,
XYZ_PIXEL_FORMATS[m
[32m+[m[32mAV_PIX_FMT_RGB24, AV_PIX_FMT_RGBA, XYZ_PIXEL_FORMATS,
AV_PIX_FMT_RGB48, AV_PIX_FMT_RGBA64, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS[m
};[m
[m
typedef struct LibOpenJPEGContext {[m
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] acvodec/lipopenjpeg: Improve XYZ color space detection
The reason for this change is that the native jpeg2000 decoder does not yet support 4K digital cinema files (#2586). The workaround for that is to use libopenjpeg decoder, which sort of works but incorrectly detects pixel format as rgb48le instead of xyz12le. This produces wrong colors in the output. This patch basically fixes the issue so that the correct output is produced when using libopenjpeg decoder. I should have included this in commit message. On Tue, Mar 3, 2015 at 12:49 AM, Michael Bradshaw mjbs...@gmail.com wrote: Is this really desirable, though? The most common case should be the default. If XYZ is the most common use case for ffmpeg/avcodec users, then sure, this is good. But I'm not convinced XYZ is more common than RGB. I believe RGB is more common than XYZ, and as such should remain the default. This only affects cases where openjpeg did not detect color space (CLRSPC_UNKNOWN) as it is most likely to be in XYZ. As far as I have figured out, openjpeg does not have proper detection for XYZ images, it decodes them all right, but the information coming from their API is just not there. When the image is in RGB, openjpeg should detect it as CLRSPC_SRGB and the behavior has not changed here. I have tested this with several files and the results were correct in all cases. This small cleanup/micro optimization is better in a different patch. It seems unrelated to testing XYZ before RGB. Same for the similar change to libopenjpeg_copyto16(). This is not an optimization, but a fix for a bug. It fixes the pixel value shift for xyz12le pixel format so that the values are properly aligned. It is not as much a detection issue, but the correct detection exposed it. I think there was no way to hit it before due to incorrect pixel format detection as rgb48le does not need bit shifts but xyz12le does. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] acvodec/lipopenjpeg: Improve XYZ color space detection
On Tue, Mar 3, 2015 at 4:35 AM, Michael Bradshaw mjbs...@gmail.com wrote: You can use the -pix_fmt option to specify the pixel format of the source. The openjpeg decoder will try that pixel format first before iterating through its (prioritized) list of pixel formats. That should provide a way to work around this (until it's fixed). First thing I tried, did not work: Option pixel_format not found. On Tue, Mar 3, 2015 at 12:49 AM, Michael Bradshaw mjbs...@gmail.com That's a good point. The downside is that it breaks autodetecting RGB in J2K/JPC (which don't have the color space info stored in the file format). Perhaps XYZ should be tried first if the file is a JP2 file (since openjpeg should have detected if it was RGB otherwise, so we can assume it's not RGB), and RGB should be tried first if it's a J2K file (since RGB is more common). That way, autodetecting RGB in J2K is not broken, and detecting XYZ in JP2 should work. Thoughts from anyone on this? The problem with trying RGB before XYZ is that libopenjpeg_matches_pix_fmt matches rgb48le before xyz12le is even considered. But I see the problem with J2K files. Maybe I can improve libopenjpeg_guess_pix_fmt so it handles all cases correctly. Okay, but it should go in a separate patch. Also, instead of doing two shifts, just add desc-comp[index].shift to adjust[index] (before the big main loop) so only one shift is needed each iteration. OK, I will make this a separate patch. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] acvodec/lipopenjpeg: Fix pixel value shift for 12-bit pixel formats
Overall, looks good to me. Maybe the + desc-comp[x].shift; part should be moved inside the FFMIN macro? Seems okay as is, though. I looked how it is implemented in jpeg200dec.c and there is no FFMIN there. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Tue, Mar 3, 2015 at 11:40 AM, Carl Eugen Hoyos ceho...@ag.or.at wrote: Doesn't this break rgb48 images? No, it does not break rgb48. They are not affected by this change. Due to the way libopenjpeg_matches_pix_fmt is implemented, rgb48 can match xyz12, but xyz12 can not match rgb48. So the order of formats matter, it should be in increasing depth. With the previous format order xyz12 was never checked, because rgb48 would have been incorrectly selected first. This line is the reason for this: 106: desc-comp[2].depth_minus1 + 1 = image-comps[2].prec Since 16 = 12, rgb48 matches and xyz12 is not even considered. I do not know, why = is used here instead of ==, but I do not want to change too much. Could you explain how I can reproduce your issue? There is a sample in https://trac.ffmpeg.org/ticket/2586 but it is black, so you wold not see the issue visually. But any digital cinema file would do, like the ones produced by OpenDCP or DCP-o-matic. The command line to reproduce is: ffmpeg -c:v libopenjpeg -i fdfcde74_excerpt_00-23.mxf Unpached ffmpeg displays pixel format as rgb48le, should be xyz12le. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Tue, Mar 3, 2015 at 2:05 PM, Carl Eugen Hoyos ceho...@ag.or.at wrote: Why don't you just move XYZ_PIXEL_FORMATS in front of the list? Then the rgb24/rgba would not be detected correctly. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] acvodec/lipopenjpeg: Fix pixel value shift for 12-bit pixel formats
On Tue, Mar 3, 2015 at 10:19 AM, Vilius Grigaliūnas vilius.grigaliu...@gmail.com wrote: This fixes pixel values not being properly shifted in libopenjpeg_copyto16 and libopenjpeg_copy_to_packed16 methods. Pixel formats like xyz12le need to be shifted by AVComponentDescriptor::shift to get the correct values. The first patches had broken formatting, these last two are the good ones. As far as I have tested, this resolves all issues with XYZ color support in libopenjpeg decoder. RGB images should work as they did before without any regressions. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] acvodec/lipopenjpeg: Improve XYZ color space detection
Input files in XYZ color space are incorrecly detected as RGB which results
in incorrect output colors.
This changes pixel format detection to try XYZ before RGB when
color space provided by libopenjepg is unknown.
---
libavcodec/libopenjpegdec.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
index 02b1ceb..23febd7 100644
--- a/libavcodec/libopenjpegdec.c
+++ b/libavcodec/libopenjpegdec.c
@@ -77,7 +77,7 @@ static const enum AVPixelFormat libopenjpeg_yuv_pix_fmts[] =
{
YUV_PIXEL_FORMATS
};
static const enum AVPixelFormat libopenjpeg_all_pix_fmts[] = {
-RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS, XYZ_PIXEL_FORMATS
+XYZ_PIXEL_FORMATS, RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS
};
typedef struct LibOpenJPEGContext {
@@ -184,10 +184,11 @@ static inline void libopenjpeg_copy_to_packed8(AVFrame
*picture, opj_image_t *im
static inline void libopenjpeg_copy_to_packed16(AVFrame *picture, opj_image_t
*image) {
uint16_t *img_ptr;
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(picture-format);
int index, x, y, c;
int adjust[4];
for (x = 0; x image-numcomps; x++)
-adjust[x] =
FFMAX(FFMIN(av_pix_fmt_desc_get(picture-format)-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0);
+adjust[x] = FFMAX(FFMIN(desc-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0);
for (y = 0; y picture-height; y++) {
index = y * picture-width;
@@ -195,7 +196,7 @@ static inline void libopenjpeg_copy_to_packed16(AVFrame
*picture, opj_image_t *i
for (x = 0; x picture-width; x++, index++)
for (c = 0; c image-numcomps; c++)
*img_ptr++ = (1 image-comps[c].prec - 1) *
image-comps[c].sgnd +
- (unsigned)image-comps[c].data[index]
adjust[c];
+ (unsigned)image-comps[c].data[index]
adjust[c] desc-comp[c].shift;
}
}
@@ -220,10 +221,11 @@ static inline void libopenjpeg_copyto8(AVFrame *picture,
opj_image_t *image) {
static inline void libopenjpeg_copyto16(AVFrame *picture, opj_image_t *image) {
int *comp_data;
uint16_t *img_ptr;
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(picture-format);
int index, x, y;
int adjust[4];
for (x = 0; x image-numcomps; x++)
-adjust[x] =
FFMAX(FFMIN(av_pix_fmt_desc_get(picture-format)-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0);
+adjust[x] = FFMAX(FFMIN(desc-comp[x].depth_minus1 + 1 -
image-comps[x].prec, 8), 0);
for (index = 0; index image-numcomps; index++) {
comp_data = image-comps[index].data;
@@ -231,7 +233,7 @@ static inline void libopenjpeg_copyto16(AVFrame *picture,
opj_image_t *image) {
img_ptr = (uint16_t *)(picture-data[index] + y *
picture-linesize[index]);
for (x = 0; x image-comps[index].w; x++) {
*img_ptr = (1 image-comps[index].prec - 1) *
image-comps[index].sgnd +
- (unsigned)*comp_data adjust[index];
+ (unsigned)*comp_data adjust[index]
desc-comp[index].shift;
img_ptr++;
comp_data++;
}
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
