[FFmpeg-devel] [PATCH] lavfi/kerndeint: use width instead of linesize.
Stefano Sabatini
stefasab at gmail.com
Sun Jan 6 11:13:59 CET 2013
On date Sunday 2013-01-06 07:58:25 +0100, Clément Bœsch encoded:
> On Sun, Jan 06, 2013 at 02:56:06AM -0300, James Almer wrote:
> > On 06/01/13 2:31 AM, Clément Bœsch wrote:
> > > On Sun, Jan 06, 2013 at 06:31:21AM +0100, Clément Bœsch wrote:
> > >> On Sun, Jan 06, 2013 at 05:57:50AM +0100, Clément Bœsch wrote:
> > >>> ---
> > >>> I may be completely wrong, but the code looks way more legit this way.
> > >>> ---
> > >>> libavfilter/vf_kerndeint.c | 28 ++++++++++++++--------------
> > >>> tests/ref/lavfi/kerndeint | 18 +++++++++---------
> > >>> 2 files changed, 23 insertions(+), 23 deletions(-)
> > >>>
> > >>
> > >> New patch attached, which should be better. But I have two questions:
> > >>
> > >> - why is pixel_step unused?
I forgot to remove it.
> > >> - AV_PIX_FMT_YUV420P is planar while AV_PIX_FMT_YUYV422 is packed, is
> > >> this correct?
YUYV is like the name suggest and is packed, yuv420p is planar of course.
> > >>
> > >
> > > Even better with the patch.
> >
> > For the record, this patch fixed the crash i was experiencing with the FATE suit (It was failing
> > when testing YUV420P).
> >
>
> Yes but that was still not correct. I think the new attached patch is
> better.
>
> --
> Clément B.
> From afb2350a13085e8f628415d98c88310a2c554dde 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.
>
> ---
> libavfilter/vf_kerndeint.c | 38 +++++++++++++++++++-------------------
> tests/ref/lavfi/kerndeint | 2 +-
> 2 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/libavfilter/vf_kerndeint.c b/libavfilter/vf_kerndeint.c
> index 26f2e49..e7bc74d 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;
> @@ -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.
Also is it still correct in case of yuv420p?
tmp_linesize[plane] seems safer to me.
> 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?
> // 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
> --
> 1.8.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
--
FFmpeg = Fascinating Friendly Mortal Problematic Elastic Gangster
More information about the ffmpeg-devel
mailing list