diff mbox series

[FFmpeg-devel,15/16] lavfi/drawutils: overhaul to improve pixel format support

Message ID 20211224030904.1196-16-rcombs@rcombs.me
State Accepted
Commit 66343e46cfb1bd85dcecc066b9bf444823bd0d82
Headers show
Series [FFmpeg-devel,01/16] swscale/output: template-ize yuv2nv12cX 10-bit and 16-bit cases | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Ridley Combs Dec. 24, 2021, 3:09 a.m. UTC
- No longer mixes u8 and u16 component accesses (this was UB)
- De-duplicated 8->16 conversion
- De-duplicated component -> plane+offset conversion
- De-duplicated planar + packed RGB
- No longer calls ff_fill_rgba_map
- Removed redundant comp_mask data member
- RGB0 and related formats no longer write an alpha value to the 0 byte
- Non-planar YA formats now work correctly
- High-bit-depth semi-planar YUV now works correctly
---
 libavfilter/drawutils.c           | 110 +++++++++++++-----------------
 libavfilter/drawutils.h           |   1 -
 tests/ref/fate/filter-pixfmts-pad |  20 +++---
 3 files changed, 58 insertions(+), 73 deletions(-)
diff mbox series

Patch

diff --git a/libavfilter/drawutils.c b/libavfilter/drawutils.c
index 5308fcbc0f..cbb2582fe0 100644
--- a/libavfilter/drawutils.c
+++ b/libavfilter/drawutils.c
@@ -137,66 +137,49 @@  int ff_draw_init(FFDrawContext *draw, enum AVPixelFormat format, unsigned flags)
     memcpy(draw->pixelstep, pixelstep, sizeof(draw->pixelstep));
     draw->hsub[1] = draw->hsub[2] = draw->hsub_max = desc->log2_chroma_w;
     draw->vsub[1] = draw->vsub[2] = draw->vsub_max = desc->log2_chroma_h;
-    for (i = 0; i < (desc->nb_components - !!(desc->flags & AV_PIX_FMT_FLAG_ALPHA && !(flags & FF_DRAW_PROCESS_ALPHA))); i++)
-        draw->comp_mask[desc->comp[i].plane] |=
-            1 << desc->comp[i].offset;
     return 0;
 }
 
 void ff_draw_color(FFDrawContext *draw, FFDrawColor *color, const uint8_t rgba[4])
 {
     unsigned i;
-    uint8_t rgba_map[4];
+    uint8_t tmp8[4];
+    const AVPixFmtDescriptor *desc = draw->desc;
 
     if (rgba != color->rgba)
         memcpy(color->rgba, rgba, sizeof(color->rgba));
-    if ((draw->desc->flags & AV_PIX_FMT_FLAG_RGB) &&
-        ff_fill_rgba_map(rgba_map, draw->format) >= 0) {
-        if (draw->nb_planes == 1) {
-            for (i = 0; i < 4; i++) {
-                color->comp[0].u8[rgba_map[i]] = rgba[i];
-                if (draw->desc->comp[rgba_map[i]].depth > 8) {
-                    color->comp[0].u16[rgba_map[i]] = color->comp[0].u8[rgba_map[i]] << 8;
-                }
-            }
-        } else {
-            for (i = 0; i < 4; i++) {
-                color->comp[rgba_map[i]].u8[0] = rgba[i];
-                if (draw->desc->comp[rgba_map[i]].depth > 8)
-                    color->comp[rgba_map[i]].u16[0] = color->comp[rgba_map[i]].u8[0] << (draw->desc->comp[rgba_map[i]].depth - 8);
-            }
-        }
+
+    memset(color->comp, 0, sizeof(color->comp));
+
+    if (draw->desc->flags & AV_PIX_FMT_FLAG_RGB) {
+        memcpy(tmp8, rgba, sizeof(tmp8));
     } else if (draw->nb_planes >= 2) {
         /* assume YUV */
-        const AVPixFmtDescriptor *desc = draw->desc;
-        color->comp[desc->comp[0].plane].u8[desc->comp[0].offset] = draw->full_range ? RGB_TO_Y_JPEG(rgba[0], rgba[1], rgba[2]) : RGB_TO_Y_CCIR(rgba[0], rgba[1], rgba[2]);
-        color->comp[desc->comp[1].plane].u8[desc->comp[1].offset] = draw->full_range ? RGB_TO_U_JPEG(rgba[0], rgba[1], rgba[2]) : RGB_TO_U_CCIR(rgba[0], rgba[1], rgba[2], 0);
-        color->comp[desc->comp[2].plane].u8[desc->comp[2].offset] = draw->full_range ? RGB_TO_V_JPEG(rgba[0], rgba[1], rgba[2]) : RGB_TO_V_CCIR(rgba[0], rgba[1], rgba[2], 0);
-        color->comp[3].u8[0] = rgba[3];
-#define EXPAND(compn) \
-        if (desc->comp[compn].depth > 8) \
-            color->comp[desc->comp[compn].plane].u16[desc->comp[compn].offset] = \
-            color->comp[desc->comp[compn].plane].u8[desc->comp[compn].offset] << \
-                (draw->desc->comp[compn].depth + draw->desc->comp[compn].shift - 8)
-        EXPAND(3);
-        EXPAND(2);
-        EXPAND(1);
-        EXPAND(0);
+        tmp8[0] = draw->full_range ? RGB_TO_Y_JPEG(rgba[0], rgba[1], rgba[2]) : RGB_TO_Y_CCIR(rgba[0], rgba[1], rgba[2]);
+        tmp8[1] = draw->full_range ? RGB_TO_U_JPEG(rgba[0], rgba[1], rgba[2]) : RGB_TO_U_CCIR(rgba[0], rgba[1], rgba[2], 0);
+        tmp8[2] = draw->full_range ? RGB_TO_V_JPEG(rgba[0], rgba[1], rgba[2]) : RGB_TO_V_CCIR(rgba[0], rgba[1], rgba[2], 0);
+        tmp8[3] = rgba[3];
     } else if (draw->format == AV_PIX_FMT_GRAY8 || draw->format == AV_PIX_FMT_GRAY8A ||
                draw->format == AV_PIX_FMT_GRAY16LE || draw->format == AV_PIX_FMT_YA16LE ||
                draw->format == AV_PIX_FMT_GRAY9LE  ||
                draw->format == AV_PIX_FMT_GRAY10LE ||
                draw->format == AV_PIX_FMT_GRAY12LE ||
                draw->format == AV_PIX_FMT_GRAY14LE) {
-        const AVPixFmtDescriptor *desc = draw->desc;
-        color->comp[0].u8[0] = RGB_TO_Y_CCIR(rgba[0], rgba[1], rgba[2]);
-        EXPAND(0);
-        color->comp[1].u8[0] = rgba[3];
-        EXPAND(1);
+        tmp8[0] = RGB_TO_Y_CCIR(rgba[0], rgba[1], rgba[2]);
+        tmp8[1] = rgba[3];
     } else {
         av_log(NULL, AV_LOG_WARNING,
                "Color conversion not implemented for %s\n", draw->desc->name);
         memset(color, 128, sizeof(*color));
+        return;
+    }
+
+    for (i = 0; i < desc->nb_components; i++) {
+        if (desc->comp[i].depth > 8)
+            color->comp[desc->comp[i].plane].u16[desc->comp[i].offset / 2] = tmp8[i] <<
+                (draw->desc->comp[i].depth + draw->desc->comp[i].shift - 8);
+        else
+            color->comp[desc->comp[i].plane].u8[desc->comp[i].offset] = tmp8[i];
     }
 }
 
@@ -302,11 +285,6 @@  static void subsampling_bounds(int sub, int *x, int *w, int *start, int *end)
     *w >>= sub;
 }
 
-static int component_used(FFDrawContext *draw, int plane, int comp)
-{
-    return (draw->comp_mask[plane] >> comp) & 1;
-}
-
 /* If alpha is in the [ 0 ; 0x1010101 ] range,
    then alpha * value is in the [ 0 ; 0xFFFFFFFF ] range,
    and >> 24 gives a correct rounding. */
@@ -366,6 +344,9 @@  void ff_blend_rectangle(FFDrawContext *draw, FFDrawColor *color,
     int w_sub, h_sub, x_sub, y_sub, left, right, top, bottom, y;
     uint8_t *p0, *p;
 
+    nb_comp = draw->desc->nb_components -
+        !!(draw->desc->flags & AV_PIX_FMT_FLAG_ALPHA && !(draw->flags & FF_DRAW_PROCESS_ALPHA));
+
     /* TODO optimize if alpha = 0xFF */
     clip_interval(dst_w, &x0, &w, NULL);
     clip_interval(dst_h, &y0, &h, NULL);
@@ -381,7 +362,6 @@  void ff_blend_rectangle(FFDrawContext *draw, FFDrawColor *color,
     nb_planes = draw->nb_planes - !!(draw->desc->flags & AV_PIX_FMT_FLAG_ALPHA && !(draw->flags & FF_DRAW_PROCESS_ALPHA));
     nb_planes += !nb_planes;
     for (plane = 0; plane < nb_planes; plane++) {
-        nb_comp = draw->pixelstep[plane];
         p0 = pointer_at(draw, dst, dst_linesize, plane, x0, y0);
         w_sub = w;
         h_sub = h;
@@ -391,17 +371,19 @@  void ff_blend_rectangle(FFDrawContext *draw, FFDrawColor *color,
         subsampling_bounds(draw->vsub[plane], &y_sub, &h_sub, &top, &bottom);
         for (comp = 0; comp < nb_comp; comp++) {
             const int depth = draw->desc->comp[comp].depth;
+            const int offset = draw->desc->comp[comp].offset;
+            const int index = offset / ((depth + 7) / 8);
 
-            if (!component_used(draw, plane, comp))
+            if (draw->desc->comp[comp].plane != plane)
                 continue;
-            p = p0 + comp;
+            p = p0 + offset;
             if (top) {
                 if (depth <= 8) {
-                    blend_line(p, color->comp[plane].u8[comp], alpha >> 1,
+                    blend_line(p, color->comp[plane].u8[index], alpha >> 1,
                                draw->pixelstep[plane], w_sub,
                                draw->hsub[plane], left, right);
                 } else {
-                    blend_line16(p, color->comp[plane].u16[comp], alpha >> 1,
+                    blend_line16(p, color->comp[plane].u16[index], alpha >> 1,
                                  draw->pixelstep[plane], w_sub,
                                  draw->hsub[plane], left, right);
                 }
@@ -409,14 +391,14 @@  void ff_blend_rectangle(FFDrawContext *draw, FFDrawColor *color,
             }
             if (depth <= 8) {
                 for (y = 0; y < h_sub; y++) {
-                    blend_line(p, color->comp[plane].u8[comp], alpha,
+                    blend_line(p, color->comp[plane].u8[index], alpha,
                                draw->pixelstep[plane], w_sub,
                                draw->hsub[plane], left, right);
                     p += dst_linesize[plane];
                 }
             } else {
                 for (y = 0; y < h_sub; y++) {
-                    blend_line16(p, color->comp[plane].u16[comp], alpha,
+                    blend_line16(p, color->comp[plane].u16[index], alpha,
                                  draw->pixelstep[plane], w_sub,
                                  draw->hsub[plane], left, right);
                     p += dst_linesize[plane];
@@ -424,11 +406,11 @@  void ff_blend_rectangle(FFDrawContext *draw, FFDrawColor *color,
             }
             if (bottom) {
                 if (depth <= 8) {
-                    blend_line(p, color->comp[plane].u8[comp], alpha >> 1,
+                    blend_line(p, color->comp[plane].u8[index], alpha >> 1,
                                draw->pixelstep[plane], w_sub,
                                draw->hsub[plane], left, right);
                 } else {
-                    blend_line16(p, color->comp[plane].u16[comp], alpha >> 1,
+                    blend_line16(p, color->comp[plane].u16[index], alpha >> 1,
                                  draw->pixelstep[plane], w_sub,
                                  draw->hsub[plane], left, right);
                 }
@@ -544,6 +526,9 @@  void ff_blend_mask(FFDrawContext *draw, FFDrawColor *color,
     uint8_t *p0, *p;
     const uint8_t *m;
 
+    nb_comp = draw->desc->nb_components -
+        !!(draw->desc->flags & AV_PIX_FMT_FLAG_ALPHA && !(draw->flags & FF_DRAW_PROCESS_ALPHA));
+
     clip_interval(dst_w, &x0, &mask_w, &xm0);
     clip_interval(dst_h, &y0, &mask_h, &ym0);
     mask += ym0 * mask_linesize;
@@ -559,7 +544,6 @@  void ff_blend_mask(FFDrawContext *draw, FFDrawColor *color,
     nb_planes = draw->nb_planes - !!(draw->desc->flags & AV_PIX_FMT_FLAG_ALPHA && !(draw->flags & FF_DRAW_PROCESS_ALPHA));
     nb_planes += !nb_planes;
     for (plane = 0; plane < nb_planes; plane++) {
-        nb_comp = draw->pixelstep[plane];
         p0 = pointer_at(draw, dst, dst_linesize, plane, x0, y0);
         w_sub = mask_w;
         h_sub = mask_h;
@@ -569,21 +553,23 @@  void ff_blend_mask(FFDrawContext *draw, FFDrawColor *color,
         subsampling_bounds(draw->vsub[plane], &y_sub, &h_sub, &top, &bottom);
         for (comp = 0; comp < nb_comp; comp++) {
             const int depth = draw->desc->comp[comp].depth;
+            const int offset = draw->desc->comp[comp].offset;
+            const int index = offset / ((depth + 7) / 8);
 
-            if (!component_used(draw, plane, comp))
+            if (draw->desc->comp[comp].plane != plane)
                 continue;
-            p = p0 + comp;
+            p = p0 + offset;
             m = mask;
             if (top) {
                 if (depth <= 8) {
                     blend_line_hv(p, draw->pixelstep[plane],
-                                  color->comp[plane].u8[comp], alpha,
+                                  color->comp[plane].u8[index], alpha,
                                   m, mask_linesize, l2depth, w_sub,
                                   draw->hsub[plane], draw->vsub[plane],
                                   xm0, left, right, top);
                 } else {
                     blend_line_hv16(p, draw->pixelstep[plane],
-                                    color->comp[plane].u16[comp], alpha,
+                                    color->comp[plane].u16[index], alpha,
                                     m, mask_linesize, l2depth, w_sub,
                                     draw->hsub[plane], draw->vsub[plane],
                                     xm0, left, right, top);
@@ -594,7 +580,7 @@  void ff_blend_mask(FFDrawContext *draw, FFDrawColor *color,
             if (depth <= 8) {
                 for (y = 0; y < h_sub; y++) {
                     blend_line_hv(p, draw->pixelstep[plane],
-                                  color->comp[plane].u8[comp], alpha,
+                                  color->comp[plane].u8[index], alpha,
                                   m, mask_linesize, l2depth, w_sub,
                                   draw->hsub[plane], draw->vsub[plane],
                                   xm0, left, right, 1 << draw->vsub[plane]);
@@ -604,7 +590,7 @@  void ff_blend_mask(FFDrawContext *draw, FFDrawColor *color,
             } else {
                 for (y = 0; y < h_sub; y++) {
                     blend_line_hv16(p, draw->pixelstep[plane],
-                                    color->comp[plane].u16[comp], alpha,
+                                    color->comp[plane].u16[index], alpha,
                                     m, mask_linesize, l2depth, w_sub,
                                     draw->hsub[plane], draw->vsub[plane],
                                     xm0, left, right, 1 << draw->vsub[plane]);
@@ -615,13 +601,13 @@  void ff_blend_mask(FFDrawContext *draw, FFDrawColor *color,
             if (bottom) {
                 if (depth <= 8) {
                     blend_line_hv(p, draw->pixelstep[plane],
-                                  color->comp[plane].u8[comp], alpha,
+                                  color->comp[plane].u8[index], alpha,
                                   m, mask_linesize, l2depth, w_sub,
                                   draw->hsub[plane], draw->vsub[plane],
                                   xm0, left, right, bottom);
                 } else {
                     blend_line_hv16(p, draw->pixelstep[plane],
-                                    color->comp[plane].u16[comp], alpha,
+                                    color->comp[plane].u16[index], alpha,
                                     m, mask_linesize, l2depth, w_sub,
                                     draw->hsub[plane], draw->vsub[plane],
                                     xm0, left, right, bottom);
diff --git a/libavfilter/drawutils.h b/libavfilter/drawutils.h
index 2ca2475585..396688514e 100644
--- a/libavfilter/drawutils.h
+++ b/libavfilter/drawutils.h
@@ -37,7 +37,6 @@  typedef struct FFDrawContext {
     enum AVPixelFormat format;
     unsigned nb_planes;
     int pixelstep[MAX_PLANES]; /*< offset between pixels */
-    uint8_t comp_mask[MAX_PLANES]; /*< bitmask of used non-alpha components */
     uint8_t hsub[MAX_PLANES];  /*< horizontal subsampling */
     uint8_t vsub[MAX_PLANES];  /*< vertical subsampling */
     uint8_t hsub_max;
diff --git a/tests/ref/fate/filter-pixfmts-pad b/tests/ref/fate/filter-pixfmts-pad
index 74981cd6c1..d8c348a0fe 100644
--- a/tests/ref/fate/filter-pixfmts-pad
+++ b/tests/ref/fate/filter-pixfmts-pad
@@ -1,8 +1,8 @@ 
-0bgr                7bc6f5a1c44cdd7506174dccf52c68d7
-0rgb                ff12e0f1e576b47a4c962729d5c0b868
+0bgr                55d41bba3609383bf658169f90b30b42
+0rgb                8e076dd0f8a9f4652595dffe3544f0f0
 abgr                52738042432893de555e6a3833172806
 argb                2a10108ac524b422b8a2393c064b3eab
-bgr0                32207a2de1b2ac7937e940a8459b97c0
+bgr0                025d4d5e5691801ba39bc9de70e39df0
 bgr24               f8b65ad845905c7d0c93ca28dfbb826f
 bgra                929aac15e848038e367c250037575f9f
 gbrap               5f16cccab5a17cb766c882e865995167
@@ -25,16 +25,16 @@  nv12                381574979cb04be10c9168540310afad
 nv21                0fdeb2cdd56cf5a7147dc273456fa217
 nv24                193b9eadcc06ad5081609f76249b3e47
 nv42                1738ad3c31c6c16e17679f5b09ce4677
-p210le              10b53de63b086de93c076d1d40f9da42
-p216le              0bbf778e1b6101a3f650ce0454a357f2
-p410le              fcab6381bde9cd84b813925ff29be4d2
-p416le              6db094f8d7d27d7299bf9496ad66e2e0
-rgb0                78d500c8361ab6423a4826a00268c908
+p210le              abc02945a9b9585f0914716e4787cefb
+p216le              1b43feb94b8a030c0c699aa0deff017b
+p410le              1f0294141ae1657d6c10c6a0d46a879f
+p416le              320e558b7ee8d598231ae0763ecca275
+rgb0                0984eb985dabbe757ed6beb53db84eff
 rgb24               17f9e2e0c609009acaf2175c42d4a2a5
 rgba                b157c90191463d34fb3ce77b36c96386
 xyz12le             85abf80b77a9236a76ba0b00fcbdea2d
-ya16le              940fafa240b9916de5f73cb20a552f24
-ya8                 5fc0f471207ddf7aa01b07027d56b672
+ya16le              d85740ba2cac9fa9ea8aaea8a5864407
+ya8                 495daaca2dcb4f7aeba7652768b41ced
 yuv410p             cb871dcc1e84a7ef1d21f9237b88cf6e
 yuv411p             aec2c1740de9a62db0d41f4dda9121b0
 yuv420p             4398e408fc35436ce4b20468946f58b6