[FFmpeg-devel] [RFC][PATCH 2/2] libopenjpegdec: rewrite check_image_attributes()

[Paul B Mahol]

Previous one was prone to segfaults and misdetection.

Signed-off-by: Paul B Mahol <onemda at gmail.com>
---
 libavcodec/libopenjpegdec.c |  118 ++++++++++++++++++++----------------------
 1 files changed, 56 insertions(+), 62 deletions(-)

diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
index 2744713..4dd8129 100644
--- a/libavcodec/libopenjpegdec.c
+++ b/libavcodec/libopenjpegdec.c
@@ -24,6 +24,8 @@
 * JPEG 2000 decoder using libopenjpeg
 */
 
+#define DEBUG
+
 #include "libavutil/imgutils.h"
 #include "libavutil/pixfmt.h"
 #include "avcodec.h"
@@ -42,66 +44,66 @@ typedef struct {
 
 static enum PixelFormat check_image_attributes(AVCodecContext *avctx, opj_image_t *image)
 {
-    opj_image_comp_t c0 = image->comps[0];
-    opj_image_comp_t c1 = image->comps[1];
-    opj_image_comp_t c2 = image->comps[2];
-    int compRatio = 0;
-    compRatio |= c0.dx << 15 | c0.dy << 12;
-    compRatio |= c1.dx << 9  | c1.dy << 6;
-    compRatio |= c2.dx << 3  | c2.dy;
+    enum PixelFormat fmt = PIX_FMT_NONE;
+    AVPixFmtDescriptor pixdesc = { 0 };
+    int i, color_space;
+    int numcomps = image->numcomps;
 
-    if (image->numcomps == 4) {
-        if (c0.prec == 8) {
-            if (compRatio == 0112222 &&
-                image->comps[3].dx == 1 && image->comps[3].dy == 1) {
-                return PIX_FMT_YUVA420P;
-            } else {
-                return PIX_FMT_RGBA;
-            }
-        } else {
-            return PIX_FMT_RGBA64;
-        }
-    }
+    if (numcomps < 1 || numcomps > 4)
+        goto fail;
+    if ((image->comps[0].dx != 1) || (image->comps[0].dy != 1))
+        goto fail;
+    if ((numcomps > 2) && (image->comps[1].dy != image->comps[2].dy))
+        goto fail;
+    if ((numcomps > 3) && (image->comps[3].dx != 1 || image->comps[3].dy != 1))
+        goto fail;
 
-    switch (compRatio) {
-    case 0111111: goto libopenjpeg_yuv444_rgb;
-    case 0111212: return PIX_FMT_YUV440P;
-    case 0112121: goto libopenjpeg_yuv422;
-    case 0112222: goto libopenjpeg_yuv420;
-    default: goto libopenjpeg_rgb;
+    av_dlog(avctx, "numcomps %d\n", numcomps);
+    for (i = 0; i < numcomps; i++) {
+        pixdesc.comp[i].depth_minus1 = image->comps[i].prec - 1;
+        av_dlog(avctx, "%d prec %d dx %d dy %d\n", i, image->comps[i].prec, image->comps[i].dx, image->comps[i].dy);
     }
 
-libopenjpeg_yuv420:
-    switch (c0.prec) {
-    case 8:  return PIX_FMT_YUV420P;
-    case 9:  return PIX_FMT_YUV420P9;
-    case 10: return PIX_FMT_YUV420P10;
-    case 16: return PIX_FMT_YUV420P16;
-    }
+    pixdesc.nb_components = image->numcomps;
+    pixdesc.log2_chroma_w = image->comps[1].dx >> 1;
+    pixdesc.log2_chroma_h = image->comps[1].dy >> 1;
 
-libopenjpeg_yuv422:
-    switch (c0.prec) {
-    case 8:  return PIX_FMT_YUV422P;
-    case 9:  return PIX_FMT_YUV422P9;
-    case 10: return PIX_FMT_YUV422P10;
-    case 16: return PIX_FMT_YUV422P16;
+    color_space = image->color_space;
+    if (color_space == CLRSPC_UNSPECIFIED) {
+        av_log(avctx, AV_LOG_WARNING, "colorspace unspecified, guessing...\n");
+        if (numcomps < 3)
+            color_space = CLRSPC_GRAY;
+        else
+            color_space = CLRSPC_SYCC;
     }
-
-libopenjpeg_yuv444_rgb:
-    switch (c0.prec) {
-    case 8:  return PIX_FMT_RGB24;
-    case 9:  return PIX_FMT_YUV444P9;
-    case 10: return PIX_FMT_YUV444P10;
-    case 16: return PIX_FMT_YUV444P16;
+    switch (color_space) {
+    case CLRSPC_SRGB:
+        pixdesc.flags |= PIX_FMT_RGB;
+        for (i = 0; i < numcomps; i++)
+            pixdesc.comp[i].offset_plus1 = 1 + i * image->comps[i].prec / 8;
+        fmt = av_find_pix_fmt(&pixdesc, 16 | 4);
+        break;
+    case CLRSPC_GRAY:
+        if (numcomps == 1 && image->comps[0].prec == 8)
+            fmt = PIX_FMT_GRAY8;
+        else if (numcomps == 2 && image->comps[0].prec == 8)
+            fmt = PIX_FMT_GRAY8A;
+        else if (numcomps == 1 && image->comps[0].prec == 16)
+            fmt = PIX_FMT_GRAY16;
+        break;
+    case CLRSPC_SYCC:
+        pixdesc.flags |= PIX_FMT_PLANAR;
+        for (i = 0; i < numcomps; i++)
+            pixdesc.comp[i].offset_plus1 = 1;
+        fmt = av_find_pix_fmt(&pixdesc, 16 | 4);
+        break;
     }
 
-libopenjpeg_rgb:
-    switch (c0.prec) {
-    case 8: return PIX_FMT_RGB24;
-    default: return PIX_FMT_RGB48;
-    }
+    if (fmt == PIX_FMT_NONE)
+fail:
+        av_log_ask_for_sample(avctx, "unsupported pixel format\n");
 
-    return PIX_FMT_RGB24;
+    return fmt;
 }
 
 static inline int libopenjpeg_ispacked(enum PixelFormat pix_fmt) {
@@ -265,17 +267,9 @@ static int libopenjpeg_decode_frame(AVCodecContext *avctx,
     }
     avcodec_set_dimensions(avctx, width, height);
 
-    switch (image->numcomps) {
-    case 1:  avctx->pix_fmt = (image->comps[0].bpp == 8) ? PIX_FMT_GRAY8 : PIX_FMT_GRAY16;
-             break;
-    case 2:  avctx->pix_fmt = PIX_FMT_GRAY8A;
-             break;
-    case 3:
-    case 4:  avctx->pix_fmt = check_image_attributes(avctx, image);
-             break;
-    default: av_log(avctx, AV_LOG_ERROR, "%d components unsupported.\n", image->numcomps);
-             goto done;
-    }
+    avctx->pix_fmt = check_image_attributes(avctx, image);
+    if (avctx->pix_fmt == PIX_FMT_NONE)
+        goto done;
 
     if(picture->data[0])
         ff_thread_release_buffer(avctx, picture);
-- 
1.7.7