diff mbox series

[FFmpeg-devel,2/4] lav/dnxhd: CID 1256 is RGB, not BGR or YUV444

Message ID 20210130091906.312-3-christophe.gisquet@gmail.com
State New
Headers show
Series Better colorspace support in dnxhddec
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

Christophe Gisquet Jan. 30, 2021, 9:19 a.m. UTC
From: Christophe Gisquet <c.gisquet@ateme.com>

Fix the logic around checking the ACT flag per MB and row.
This also requires adding a 444 path to swap channels into
the ffmpeg formats, as they are GBR, and not RGB.
---
 libavcodec/dnxhddec.c | 64 +++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 17 deletions(-)

Comments

Michael Niedermayer Jan. 31, 2021, 1:11 p.m. UTC | #1
On Sat, Jan 30, 2021 at 09:19:04AM +0000, Christophe Gisquet wrote:
> From: Christophe Gisquet <c.gisquet@ateme.com>
> 
> Fix the logic around checking the ACT flag per MB and row.
> This also requires adding a 444 path to swap channels into
> the ffmpeg formats, as they are GBR, and not RGB.
> ---
>  libavcodec/dnxhddec.c | 64 +++++++++++++++++++++++++++++++------------
>  1 file changed, 47 insertions(+), 17 deletions(-)

This transmutes the following dog into a hyperspace neon dog
./ffplay DNxHDtest2.mov

sample probably here: https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket3069/

thx

[...]
Christophe Gisquet Feb. 1, 2021, 6 p.m. UTC | #2
Hi,

Le dim. 31 janv. 2021 à 14:11, Michael Niedermayer
<michael@niedermayer.cc> a écrit :
> This transmutes the following dog into a hyperspace neon dog
> ./ffplay DNxHDtest2.mov

I'm not sure I prefer the correct version, but here goes. This sample
is YUV444 basically, the reverse of what I've seen in another sample.
diff mbox series

Patch

diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c
index 359588f963..11da1c286c 100644
--- a/libavcodec/dnxhddec.c
+++ b/libavcodec/dnxhddec.c
@@ -249,12 +249,10 @@  static int dnxhd_decode_header(DNXHDContext *ctx, AVFrame *frame,
             return AVERROR_INVALIDDATA;
         } else if (bitdepth == 10) {
             ctx->decode_dct_block = dnxhd_decode_dct_block_10_444;
-            ctx->pix_fmt = ctx->act ? AV_PIX_FMT_YUV444P10
-                                    : AV_PIX_FMT_GBRP10;
+            ctx->pix_fmt = ctx->act ? AV_PIX_FMT_GBRP10 : AV_PIX_FMT_YUV444P10;
         } else {
             ctx->decode_dct_block = dnxhd_decode_dct_block_12_444;
-            ctx->pix_fmt = ctx->act ? AV_PIX_FMT_YUV444P12
-                                    : AV_PIX_FMT_GBRP12;
+            ctx->pix_fmt = ctx->act ? AV_PIX_FMT_GBRP12 : AV_PIX_FMT_YUV444P12;
         }
     } else if (bitdepth == 12) {
         ctx->decode_dct_block = dnxhd_decode_dct_block_12;
@@ -504,19 +502,19 @@  static int dnxhd_decode_macroblock(const DNXHDContext *ctx, RowContext *row,
         qscale = get_bits(&row->gb, 11);
     }
     act = get_bits1(&row->gb);
-    if (act) {
-        if (!ctx->act) {
-            static int act_warned;
-            if (!act_warned) {
-                act_warned = 1;
-                av_log(ctx->avctx, AV_LOG_ERROR,
-                       "ACT flag set, in violation of frame header.\n");
-            }
-        } else if (row->format == -1) {
+    if (ctx->act) {
+        if (row->format == -1) {
             row->format = act;
         } else if (row->format != act) {
             row->format = 2; // Variable
         }
+    } else if (act) {
+        static int act_warned;
+        if (!act_warned) {
+            act_warned = 1;
+            av_log(ctx->avctx, AV_LOG_ERROR,
+                   "ACT flag set, in violation of frame header.\n");
+        }
     }
 
     if (qscale != row->last_qscale) {
@@ -569,6 +567,21 @@  static int dnxhd_decode_macroblock(const DNXHDContext *ctx, RowContext *row,
         }
         break;
     case DNX_CHROMAFORMAT_444:
+        if (ctx->avctx->profile == FF_PROFILE_DNXHD) {
+            ctx->idsp.idct_put(dest_y,                               dct_linesize_luma, row->blocks[2]);
+            ctx->idsp.idct_put(dest_y + dct_x_offset,                dct_linesize_luma, row->blocks[3]);
+            ctx->idsp.idct_put(dest_y + dct_y_offset,                dct_linesize_luma, row->blocks[8]);
+            ctx->idsp.idct_put(dest_y + dct_y_offset + dct_x_offset, dct_linesize_luma, row->blocks[9]);
+
+            ctx->idsp.idct_put(dest_u,                               dct_linesize_luma, row->blocks[4]);
+            ctx->idsp.idct_put(dest_u + dct_x_offset,                dct_linesize_luma, row->blocks[5]);
+            ctx->idsp.idct_put(dest_u + dct_y_offset,                dct_linesize_luma, row->blocks[10]);
+            ctx->idsp.idct_put(dest_u + dct_y_offset + dct_x_offset, dct_linesize_luma, row->blocks[11]);
+            ctx->idsp.idct_put(dest_v,                               dct_linesize_luma, row->blocks[0]);
+            ctx->idsp.idct_put(dest_v + dct_x_offset,                dct_linesize_luma, row->blocks[1]);
+            ctx->idsp.idct_put(dest_v + dct_y_offset,                dct_linesize_luma, row->blocks[6]);
+            ctx->idsp.idct_put(dest_v + dct_y_offset + dct_x_offset, dct_linesize_luma, row->blocks[7]);
+        } else {
         ctx->idsp.idct_put(dest_y,                               dct_linesize_luma, row->blocks[0]);
         ctx->idsp.idct_put(dest_y + dct_x_offset,                dct_linesize_luma, row->blocks[1]);
         ctx->idsp.idct_put(dest_y + dct_y_offset,                dct_linesize_luma, row->blocks[6]);
@@ -585,6 +598,7 @@  static int dnxhd_decode_macroblock(const DNXHDContext *ctx, RowContext *row,
             ctx->idsp.idct_put(dest_v + dct_y_offset,                dct_linesize_chroma, row->blocks[10]);
             ctx->idsp.idct_put(dest_v + dct_y_offset + dct_x_offset, dct_linesize_chroma, row->blocks[11]);
         }
+        }
         break;
     case DNX_CHROMAFORMAT_420:
         ctx->idsp.idct_put(dest_y,                               dct_linesize_luma, row->blocks[0]);
@@ -610,6 +624,8 @@  static int dnxhd_decode_row(AVCodecContext *avctx, void *data,
     RowContext *row = ctx->rows + threadnb;
     int x, ret;
 
+    row->format = -1;
+
     row->last_dc[0] =
     row->last_dc[1] =
     row->last_dc[2] = 1 << (ctx->bit_depth + 2); // for levels +2^(bitdepth-1)
@@ -694,14 +710,21 @@  decode_coding_unit:
         static int act_warned;
         int format = ctx->rows[0].format;
         for (i = 1; i < avctx->thread_count; i++) {
-            if (ctx->rows[i].format != format &&
-                ctx->rows[i].format != -1 /* not run */) {
+            if (ctx->rows[i].format == -1)
+                continue;
+            if (format == -1) {
+                format = ctx->rows[i].format;
+                continue;
+            }
+            if (ctx->rows[i].format != format) {
+                av_log(ctx->avctx, AV_LOG_ERROR,
+                       "Format changed from %d to %d.\n", format, ctx->rows[i].format);
                 format = 2;
                 break;
             }
         }
+
         switch (format) {
-        case -1:
         case 2:
             if (!act_warned) {
                 act_warned = 1;
@@ -709,6 +732,7 @@  decode_coding_unit:
                        "Unsupported: variable ACT flag.\n");
             }
             break;
+        case -1:
         case 0:
             ctx->pix_fmt = ctx->bit_depth==10
                          ? AV_PIX_FMT_GBRP10 : AV_PIX_FMT_GBRP12;
@@ -719,12 +743,18 @@  decode_coding_unit:
             break;
         }
     }
-    avctx->pix_fmt = ctx->pix_fmt;
     if (ret) {
         av_log(ctx->avctx, AV_LOG_ERROR, "%d lines with errors\n", ret);
         return AVERROR_INVALIDDATA;
     }
 
+    if (avctx->pix_fmt != ctx->pix_fmt) {
+        picture->format = avctx->pix_fmt = ctx->pix_fmt;
+        ret = ff_set_dimensions(avctx, ctx->width, ctx->height);
+        if (ret < 0)
+            return ret;
+    }
+
     *got_frame = 1;
     return avpkt->size;
 }