[FFmpeg-devel] [PATCH] acvodec/lipopenjpeg: Improve XYZ color space detection

[Michael Bradshaw]

On Mon, Mar 2, 2015 at 4:48 PM, Vilius Grigaliūnas <
vilius.grigaliunas at gmail.com> wrote:

> 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.
>

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).


>
> On Tue, Mar 3, 2015 at 12:49 AM, Michael Bradshaw <mjbshaw at 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.
>

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?


>
> > 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.
>

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.