[FFmpeg-devel] [PATCH] lavfi/kerndeint: use width instead of linesize.

Clément Bœsch ubitux at gmail.com
Sun Jan 6 17:50:31 CET 2013


On Sun, Jan 06, 2013 at 11:13:59AM +0100, Stefano Sabatini wrote:
[...]
> > @@ -101,9 +101,10 @@ static int config_props(AVFilterLink *inlink)
> >      int ret;
> >  
> >      kerndeint->vsub = desc->log2_chroma_h;
> > -    kerndeint->pixel_step = av_get_bits_per_pixel(desc) >> 3;
> > +    kerndeint->hsub = desc->log2_chroma_w;
> > +    kerndeint->pixel_step = av_get_padded_bits_per_pixel(desc) >> 3;
> >  
> > -    ret = av_image_alloc(kerndeint->tmp_data, kerndeint->tmp_bwidth,
> > +    ret = av_image_alloc(kerndeint->tmp_data, kerndeint->tmp_linesize,
> >                            inlink->w, inlink->h, inlink->format, 1);
> >      if (ret < 0)
> >          return ret;
> > @@ -137,8 +138,8 @@ static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
> >      uint8_t *dstp, *dstp_saved;
> >      const uint8_t *srcp_saved;
> >  
> > -    int src_linesize, psrc_linesize, dst_linesize, bwidth;
> > -    int x, y, plane, val, hi, lo, g, h, n = kerndeint->frame++;
> > +    int src_linesize, psrc_linesize, dst_linesize;
> > +    int x, y, plane, val, hi, lo, w, h, n = kerndeint->frame++;
> >      double valf;
> >  
> >      const int thresh = kerndeint->thresh;
> > @@ -159,27 +160,27 @@ static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
> >  
> >      for (plane = 0; inpic->data[plane] && plane < 4; plane++) {
> >          h = plane == 0 ? inlink->h : inlink->h >> kerndeint->vsub;
> > -        bwidth = kerndeint->tmp_bwidth[plane];
> 
> > +        w = (inlink->w * kerndeint->pixel_step) >> (plane == 0 ? 0 : kerndeint->hsub);
> 
> Honestly i don't like this, this is not a "width" but a bytewidth.

Yep sorry, restored to bwidth; I was doing various experiment and did some
rename…

> Also is it still correct in case of yuv420p?

Yes, it actually fix yuv420p. pixel_step is 1 in the case of yuv420p, so
you end up with bwidth = inlink->w >> (plane == 0 ? 0 : kerndeint->hsub),
which is more correct that the current code.

> tmp_linesize[plane] seems safer to me.
> 

You can, since the read/write is based on it.

> >          srcp = srcp_saved = inpic->data[plane];
> >          src_linesize      = inpic->linesize[plane];
> > -        psrc_linesize     = outpic->linesize[plane];
> > +        psrc_linesize     = kerndeint->tmp_linesize[plane];
> 
> this should be correct.
> 
> >          dstp = dstp_saved = outpic->data[plane];
> >          dst_linesize      = outpic->linesize[plane];
> >          srcp              = srcp_saved + (1 - order) * src_linesize;
> >          dstp              = dstp_saved + (1 - order) * dst_linesize;
> >  
> >          for (y = 0; y < h; y += 2) {
> > -            memcpy(dstp, srcp, bwidth);
> > +            memcpy(dstp, srcp, w);
> >              srcp += 2 * src_linesize;
> >              dstp += 2 * dst_linesize;
> >          }
> >  
> >          // Copy through the lines that will be missed below.
> > -        memcpy(dstp_saved + order            * dst_linesize, srcp_saved + (1 -     order) * src_linesize, bwidth);
> > -        memcpy(dstp_saved + (2 + order    )  * dst_linesize, srcp_saved + (3 -     order) * src_linesize, bwidth);
> > -        memcpy(dstp_saved + (h - 2 + order)  * dst_linesize, srcp_saved + (h - 1 - order) * src_linesize, bwidth);
> > -        memcpy(dstp_saved + (h - 4 + order)  * dst_linesize, srcp_saved + (h - 3 - order) * src_linesize, bwidth);
> > +        memcpy(dstp_saved + order            * dst_linesize, srcp_saved + (1 -     order) * src_linesize, w);
> > +        memcpy(dstp_saved + (2 + order    )  * dst_linesize, srcp_saved + (3 -     order) * src_linesize, w);
> > +        memcpy(dstp_saved + (h - 2 + order)  * dst_linesize, srcp_saved + (h - 1 - order) * src_linesize, w);
> > +        memcpy(dstp_saved + (h - 4 + order)  * dst_linesize, srcp_saved + (h - 3 - order) * src_linesize, w);
> >  
> >          /* For the other field choose adaptively between using the previous field
> >             or the interpolant from the current field. */
> > @@ -205,18 +206,17 @@ static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
> >          dstp   = dstp_saved + 5 * dst_linesize - (1 - order) * dst_linesize;
> >  
> >          for (y = 5 - (1 - order); y <= h - 5 - (1 - order); y += 2) {
> > -            for (x = 0; x < bwidth; x++) {
> > +            for (x = 0; x < w; x++) {
> >                  if (thresh == 0 || n == 0 ||
> >                      (abs((int)prvp[x]  - (int)srcp[x])  > thresh) ||
> >                      (abs((int)prvpp[x] - (int)srcpp[x]) > thresh) ||
> >                      (abs((int)prvpn[x] - (int)srcpn[x]) > thresh)) {
> >                      if (map) {
> > -                        g = x & ~3;
> > -
> >                          if (is_packed_rgb) {
> > -                            AV_WB32(dstp + g, 0xffffffff);
> > -                            x = g + 3;
> > +                            AV_WB32(dstp + x, 0xffffffff);
> > +                            x += kerndeint->pixel_step - 1;
> >                          } else if (inlink->format == AV_PIX_FMT_YUYV422) {
> > +                            int g = x & ~3;
> 
> Possibly unneeded?
> 

Not really…

> >                              // y <- 235, u <- 128, y <- 235, v <- 128
> >                              AV_WB32(dstp + g, 0xeb80eb80);
> >                              x = g + 3;
> > @@ -287,7 +287,7 @@ static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
> >  
> >          srcp = inpic->data[plane];
> >          dstp = kerndeint->tmp_data[plane];
> > -        av_image_copy_plane(dstp, psrc_linesize, srcp, src_linesize, bwidth, h);
> > +        av_image_copy_plane(dstp, psrc_linesize, srcp, src_linesize, w, h);
> >      }
> >  
> >      avfilter_unref_buffer(inpic);
> > diff --git a/tests/ref/lavfi/kerndeint b/tests/ref/lavfi/kerndeint
> > index b03f977..2769319 100644
> > --- a/tests/ref/lavfi/kerndeint
> > +++ b/tests/ref/lavfi/kerndeint
> > @@ -6,5 +6,5 @@ bgr0                364b8bcd1c7a384902077bc7190c5ea3
> >  bgra                81ac8315a4c66e363bc6fa3e99d9cd2b
> >  rgb0                ae0c2afbc266345c1372276755595105
> >  rgba                42a6cc9b815ca0ee69c29db3616ce25e
> > -yuv420p             a935cce07c5287b92c6d5220361866ed
> > +yuv420p             40ca042814882b0b791cbec38e289702
> >  yuyv422             f549c98059ba9ce50e28204256d13b5d

Simplified patch attached.

[...]

-- 
Clément B.
-------------- next part --------------
From bd2d9bb00d8e447330ab2b6be37321d683b98ff4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Sun, 6 Jan 2013 07:23:43 +0100
Subject: [PATCH] lavfi/kerndeint: fix various width/linesize bugs.

Also re-enable the FATE test.
---
 libavfilter/vf_kerndeint.c | 13 +++++++------
 tests/fate/avfilter.mak    |  1 +
 tests/ref/lavfi/kerndeint  |  2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/libavfilter/vf_kerndeint.c b/libavfilter/vf_kerndeint.c
index 26f2e49..7f162a2 100644
--- a/libavfilter/vf_kerndeint.c
+++ b/libavfilter/vf_kerndeint.c
@@ -39,9 +39,9 @@ typedef struct {
     const AVClass *class;
     int           frame; ///< frame count, starting from 0
     int           thresh, map, order, sharp, twoway;
-    int           vsub;
+    int           vsub, hsub;
     uint8_t       *tmp_data [4];  ///< temporary plane data buffer
-    int           tmp_bwidth[4];  ///< temporary plane byte width
+    int           tmp_linesize[4];
     int           pixel_step;
 } KerndeintContext;
 
@@ -101,9 +101,10 @@ static int config_props(AVFilterLink *inlink)
     int ret;
 
     kerndeint->vsub = desc->log2_chroma_h;
-    kerndeint->pixel_step = av_get_bits_per_pixel(desc) >> 3;
+    kerndeint->hsub = desc->log2_chroma_w;
+    kerndeint->pixel_step = av_get_padded_bits_per_pixel(desc) >> 3;
 
-    ret = av_image_alloc(kerndeint->tmp_data, kerndeint->tmp_bwidth,
+    ret = av_image_alloc(kerndeint->tmp_data, kerndeint->tmp_linesize,
                           inlink->w, inlink->h, inlink->format, 1);
     if (ret < 0)
         return ret;
@@ -159,11 +160,11 @@ static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
 
     for (plane = 0; inpic->data[plane] && plane < 4; plane++) {
         h = plane == 0 ? inlink->h : inlink->h >> kerndeint->vsub;
-        bwidth = kerndeint->tmp_bwidth[plane];
+        bwidth = (inlink->w * kerndeint->pixel_step) >> (plane == 0 ? 0 : kerndeint->hsub);
 
         srcp = srcp_saved = inpic->data[plane];
         src_linesize      = inpic->linesize[plane];
-        psrc_linesize     = outpic->linesize[plane];
+        psrc_linesize     = kerndeint->tmp_linesize[plane];
         dstp = dstp_saved = outpic->data[plane];
         dst_linesize      = outpic->linesize[plane];
         srcp              = srcp_saved + (1 - order) * src_linesize;
diff --git a/tests/fate/avfilter.mak b/tests/fate/avfilter.mak
index e69d6a1..e1e7280 100644
--- a/tests/fate/avfilter.mak
+++ b/tests/fate/avfilter.mak
@@ -41,6 +41,7 @@ FATE_LAVFI = fate-lavfi-alphaextract_rgb                                \
 FATE_LAVFI-$(CONFIG_GPL) += fate-lavfi-colormatrix1                     \
                             fate-lavfi-colormatrix2                     \
                             fate-lavfi-hue                              \
+                            fate-lavfi-kerndeint                        \
                             fate-lavfi-pixfmts_super2xsai               \
                             fate-lavfi-tinterlace_merge                 \
                             fate-lavfi-tinterlace_pad                   \
diff --git a/tests/ref/lavfi/kerndeint b/tests/ref/lavfi/kerndeint
index b03f977..2769319 100644
--- a/tests/ref/lavfi/kerndeint
+++ b/tests/ref/lavfi/kerndeint
@@ -6,5 +6,5 @@ bgr0                364b8bcd1c7a384902077bc7190c5ea3
 bgra                81ac8315a4c66e363bc6fa3e99d9cd2b
 rgb0                ae0c2afbc266345c1372276755595105
 rgba                42a6cc9b815ca0ee69c29db3616ce25e
-yuv420p             a935cce07c5287b92c6d5220361866ed
+yuv420p             40ca042814882b0b791cbec38e289702
 yuyv422             f549c98059ba9ce50e28204256d13b5d
-- 
1.8.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130106/ad7486fc/attachment.asc>


More information about the ffmpeg-devel mailing list