Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 04, 2015 at 05:07:17PM +0200, Vilius Grigaliūnas wrote: 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. yes, i agree but needing to manually override any sizeable amount of real world files is not very elegant [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: Digital signature ___ 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 04, 2015 at 10:08:56AM +0200, Vilius Grigaliūnas 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. 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 ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: Digital signature ___ 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 04, 2015 at 04:10:39PM +0200, Vilius Grigaliūnas wrote: 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. i did not mean to suggest to leave libopenjpeg broken. 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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: Digital signature ___ 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
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Tue, Mar 3, 2015 at 3:04 AM, Vilius Grigaliūnas vilius.grigaliu...@gmail.com wrote: 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. = was used because some images can have a weird bit depth. For example, you can make a rgb image with 9 bits per component. ffmpeg doesn't support rgb27 (only rgb24/rgb48). So = was used so we can decode weird bit depths and stuff them into a frame without losing precision (so an rgb27 would get stuffed in an rgb48 frame). ___ 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 1:19 AM, Vilius Grigaliūnas
vilius.grigaliu...@gmail.com wrote:
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
That seems like the least intrusive fix, and the logic looks right to me. I
don't know if 8-bit per component XYZ will ever be supported in ffmpeg, but
if so then deciding between rgb24 and xyz8 will need a different method.
The mixing of AV_PIX_FMT_* enums and *_PIXEL_FORMATS macros looks
tedious/fragile.
I'd say the above patch is okay. Alternatively, something like the
following could be done, I think:
diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
index 02b1ceb..da53e05 100644
--- a/libavcodec/libopenjpegdec.c
+++ b/libavcodec/libopenjpegdec.c
@@ -76,7 +76,23 @@ static const enum AVPixelFormat
libopenjpeg_gray_pix_fmts[] = {
static const enum AVPixelFormat libopenjpeg_yuv_pix_fmts[] = {
YUV_PIXEL_FORMATS
};
-static const enum AVPixelFormat libopenjpeg_all_pix_fmts[] = {
+// OpenJPEG currently doesn't support detecting xyz. If the file is
+// a JP2 file, then it should have the color space saved, and
+// OpenJPEG should be able to detect what it is. In the event that
+// the file is a JP2 and OpenJPEG can't detect what the color space
+// is, then we should try guessing xyz first, as that's the only
+// likely candidate (since OpenJPEG can detect gray/yuv/rgb, we can
+// effectively rule those out). If the file is a J2K codestream,
+// then the color format isn't known and must be guessed. In that
+// event, rgb should be guessed first, as it tends to be more
+// common. Hence, we have two arrays that prioritize search order
+// for pixel formats, based on the image type: for JP2 files,
+// libopenjpeg_jp2_all_pix_fmts; and for J2K files,
+// libopenjpeg_j2k_all_pix_fmts.
+static const enum AVPixelFormat libopenjpeg_jp2_all_pix_fmts[] = {
+XYZ_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS,
RGB_PIXEL_FORMATS
+};
+static const enum AVPixelFormat libopenjpeg_j2k_all_pix_fmts[] = {
RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS,
XYZ_PIXEL_FORMATS
};
@@ -123,7 +139,7 @@ static inline int libopenjpeg_matches_pix_fmt(const
opj_image_t *image, enum AVP
return match;
}
-static inline enum AVPixelFormat libopenjpeg_guess_pix_fmt(const
opj_image_t *image) {
+static inline enum AVPixelFormat libopenjpeg_guess_pix_fmt(const
opj_image_t *image, int is_jp2) {
int index;
const enum AVPixelFormat *possible_fmts = NULL;
int possible_fmts_nb = 0;
@@ -142,8 +158,13 @@ static inline enum AVPixelFormat
libopenjpeg_guess_pix_fmt(const opj_image_t *im
possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_yuv_pix_fmts);
break;
default:
-possible_fmts= libopenjpeg_all_pix_fmts;
-possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_all_pix_fmts);
+if (is_jp2) {
+possible_fmts= libopenjpeg_jp2_all_pix_fmts;
+possible_fmts_nb =
FF_ARRAY_ELEMS(libopenjpeg_jp2_all_pix_fmts);
+} else {
+possible_fmts= libopenjpeg_j2k_all_pix_fmts;
+possible_fmts_nb =
FF_ARRAY_ELEMS(libopenjpeg_j2k_all_pix_fmts);
+}
break;
}
@@ -263,6 +284,7 @@ static int libopenjpeg_decode_frame(AVCodecContext
*avctx,
int width, height, ret;
int pixel_size = 0;
int ispacked = 0;
+int is_jp2;
int i;
*got_frame = 0;
@@ -272,12 +294,14 @@ static int libopenjpeg_decode_frame(AVCodecContext
*avctx,
(AV_RB32(buf + 4) == JP2_SIG_TYPE)
(AV_RB32(buf + 8) == JP2_SIG_VALUE)) {
dec = opj_create_decompress(CODEC_JP2);
+is_jp2 = 1;
} else {
/* If the AVPacket contains a jp2c box, then skip to
* the starting byte of the codestream. */
if (AV_RB32(buf + 4) == AV_RB32(jp2c))
buf += 8;
dec = opj_create_decompress(CODEC_J2K);
+is_jp2 = 0;
}
if (!dec) {
@@ -320,7 +344,7 @@ static int libopenjpeg_decode_frame(AVCodecContext
*avctx,
avctx-pix_fmt = AV_PIX_FMT_NONE;
if (avctx-pix_fmt == AV_PIX_FMT_NONE)
-
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Tue, Mar 03, 2015 at 10:19:48AM +0200, Vilius Grigaliūnas wrote: 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(-) this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato signature.asc Description: Digital signature ___ 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
Vilius Grigaliūnas vilius.grigaliunas at gmail.com writes:
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
Why don't you just move XYZ_PIXEL_FORMATS in front
of the list?
Carl Eugen
___
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 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
Vilius Grigaliūnas vilius.grigaliunas at gmail.com writes: Input files in XYZ color space are incorrecly detected as RGB which results in incorrect output colors. Doesn't this break rgb48 images? Could you explain how I can reproduce your issue? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
