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) - avctx->pix_fmt = libopenjpeg_guess_pix_fmt(image); + avctx->pix_fmt = libopenjpeg_guess_pix_fmt(image, is_jp2); if (avctx->pix_fmt == AV_PIX_FMT_NONE) { av_log(avctx, AV_LOG_ERROR, "Unable to determine pixel format\n"); _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel