diff mbox series

[FFmpeg-devel] avcodec/libopenjpeg: Interpret cinema profiles as XYZ

Message ID 20210606062732.90683-1-val.zapod.vz@gmail.com
State New
Headers show
Series [FFmpeg-devel] avcodec/libopenjpeg: Interpret cinema profiles as XYZ
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Valerii Zapodovnikov June 6, 2021, 6:27 a.m. UTC
From: Rémi Achard <remiachard@gmail.com>

Patch should be applied to decode XYZ samples with not native
decoder in ffmpeg (-c:v libopenjpeg, not -c:v jpeg2000). jpeg2000
works already.
Now, this is AFAIK a patch that should be applied after upstream's
patch: https://github.com/uclouvain/openjpeg/pull/1200
Please note that jpeg2000 XYZ is not bitperfect compared to openjpeg
see #4829#comment:3. Some pixels will be different, like in sYCC. I
consider both bugs. Also pixel_format can be forced by
"-pixel_format xyz12le".
---
 libavcodec/libopenjpegdec.c | 44 +++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 17 deletions(-)

Comments

Michael Niedermayer June 6, 2021, 2:41 p.m. UTC | #1
On Sun, Jun 06, 2021 at 09:27:32AM +0300, Valerii Zapodovnikov wrote:
> From: Rémi Achard <remiachard@gmail.com>
> 
> Patch should be applied to decode XYZ samples with not native
> decoder in ffmpeg (-c:v libopenjpeg, not -c:v jpeg2000). jpeg2000
> works already.
> Now, this is AFAIK a patch that should be applied after upstream's
> patch: https://github.com/uclouvain/openjpeg/pull/1200
> Please note that jpeg2000 XYZ is not bitperfect compared to openjpeg
> see #4829#comment:3. Some pixels will be different, like in sYCC. I
> consider both bugs. Also pixel_format can be forced by
> "-pixel_format xyz12le".
> ---
>  libavcodec/libopenjpegdec.c | 44 +++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 17 deletions(-)

This breaks build

libavcodec/libopenjpegdec.c: In function ‘libopenjpeg_guess_pix_fmt’:
libavcodec/libopenjpegdec.c:203:14: error: ‘opj_image_t {aka const struct opj_image}’ has no member named ‘rsiz’
     if (image->rsiz == FF_PROFILE_JPEG2000_DCINEMA_2K ||
              ^~
libavcodec/libopenjpegdec.c:204:14: error: ‘opj_image_t {aka const struct opj_image}’ has no member named ‘rsiz’
         image->rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K) {
              ^~
ffbuild/common.mak:67: recipe for target 'libavcodec/libopenjpegdec.o' failed
make: *** [libavcodec/libopenjpegdec.o] Error 1

changes to configure, version / feature checks are missing

thx

[...]
Valerii Zapodovnikov June 6, 2021, 2:42 p.m. UTC | #2
Did you even read the commit message? You need to apply a patch to openjpeg
itself. Sigh.
Michael Niedermayer June 6, 2021, 2:53 p.m. UTC | #3
On Sun, Jun 06, 2021 at 05:42:54PM +0300, Valerii Zapodovnikov wrote:
> Did you even read the commit message? You need to apply a patch to openjpeg
> itself. Sigh.

yes i read your commit message.
you are changing the API/ABI of openjpeg and doing a matching change in
the interface in ffmpeg to openjpeg.
breaking support for the previous version with no check (not ok)
breaking support for all existing distributions (not ok)

thx

[...]
Valerii Zapodovnikov June 6, 2021, 3:01 p.m. UTC | #4
Actually it is not my patch (as should be obvious as From: is not me) and
*NO* changes in API/ABI of openjpeg are happening with upsream patch. As
for your ABI... There is only one correct way to manipulate openjpeg API,
if you did that wrong, it is your problem.

Standard thing: "unexpected that user code would allocate it with
sizeof(opj_image_t)."

I am just notifying your on this. Upstream is still not ready even, LOL.
diff mbox series

Patch

diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
index 8982d21be4..dff19586bb 100644
--- a/libavcodec/libopenjpegdec.c
+++ b/libavcodec/libopenjpegdec.c
@@ -71,6 +71,10 @@  static const enum AVPixelFormat libopenjpeg_gray_pix_fmts[] = {
 static const enum AVPixelFormat libopenjpeg_yuv_pix_fmts[]  = {
     YUV_PIXEL_FORMATS
 };
+static const enum AVPixelFormat libopenjpeg_xyz_pix_fmts[]  = {
+    XYZ_PIXEL_FORMATS,
+    YUV_PIXEL_FORMATS
+};
 static const enum AVPixelFormat libopenjpeg_all_pix_fmts[]  = {
     RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS, XYZ_PIXEL_FORMATS
 };
@@ -196,23 +200,29 @@  static inline enum AVPixelFormat libopenjpeg_guess_pix_fmt(const opj_image_t *im
     const enum AVPixelFormat *possible_fmts = NULL;
     int possible_fmts_nb = 0;
 
-    switch (image->color_space) {
-    case OPJ_CLRSPC_SRGB:
-        possible_fmts    = libopenjpeg_rgb_pix_fmts;
-        possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_rgb_pix_fmts);
-        break;
-    case OPJ_CLRSPC_GRAY:
-        possible_fmts    = libopenjpeg_gray_pix_fmts;
-        possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_gray_pix_fmts);
-        break;
-    case OPJ_CLRSPC_SYCC:
-        possible_fmts    = libopenjpeg_yuv_pix_fmts;
-        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);
-        break;
+    if (image->rsiz == FF_PROFILE_JPEG2000_DCINEMA_2K ||
+        image->rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K) {
+        possible_fmts = libopenjpeg_xyz_pix_fmts;
+        possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_xyz_pix_fmts);
+    } else {
+        switch (image->color_space) {
+        case OPJ_CLRSPC_SRGB:
+            possible_fmts    = libopenjpeg_rgb_pix_fmts;
+            possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_rgb_pix_fmts);
+            break;
+        case OPJ_CLRSPC_GRAY:
+            possible_fmts    = libopenjpeg_gray_pix_fmts;
+            possible_fmts_nb = FF_ARRAY_ELEMS(libopenjpeg_gray_pix_fmts);
+            break;
+        case OPJ_CLRSPC_SYCC:
+            possible_fmts    = libopenjpeg_yuv_pix_fmts;
+            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);
+            break;
+        }
     }
 
     for (index = 0; index < possible_fmts_nb; ++index)