[FFmpeg-devel] New mode for showwaves filter

Nicolas George george at nsup.org
Fri Mar 21 15:57:29 CET 2014


Le decadi 30 ventôse, an CCXXII, mrskman a écrit :
> Hi!
> 
> If you generate a wave with showwave filter set to mode=point, you can get
> a result which is almost invisible because of high resolution (points are
> too far from each other).
> 
> I suggest to add new mode=p2p which draws a line between those points and
> makes output more readable.
> 
> I tried to do it on my own and here is the result:
> 
> Example 1 - http://www.datafilehost.com/d/5b822974 (mode=point on
> top, mode=p2p on bottom)
> Example 2 - http://www.datafilehost.com/d/f0457d43 (mode=p2p standalone)
> 
> It's not perfect but it seems better to me. I would be glad if anyone can
> look at it or even improve it.
> 
> Thank you for any reply.

> diff --git a/libavfilter/avf_showwaves.c b/libavfilter/avf_showwaves.c
> index 0b45bd0..4dcbe84 100644
> --- a/libavfilter/avf_showwaves.c
> +++ b/libavfilter/avf_showwaves.c
> @@ -35,9 +35,21 @@
>  enum ShowWavesMode {
>      MODE_POINT,
>      MODE_LINE,
> +    MODE_P2P,
>      MODE_NB,
>  };
>  
> +enum ShowWavesOctant {
> +    OCTANT_N,
> +    OCTANT_NE,
> +    OCTANT_E,
> +    OCTANT_SE,
> +    OCTANT_S,
> +    OCTANT_SW,
> +    OCTANT_W,
> +    OCTANT_NW,
> +};
> +
>  typedef struct {
>      const AVClass *class;
>      int w, h;
> @@ -59,6 +71,7 @@ static const AVOption showwaves_options[] = {
>      { "mode", "select display mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=MODE_POINT}, 0, MODE_NB-1, FLAGS, "mode"},
>          { "point", "draw a point for each sample", 0, AV_OPT_TYPE_CONST, {.i64=MODE_POINT}, .flags=FLAGS, .unit="mode"},
>          { "line",  "draw a line for each sample",  0, AV_OPT_TYPE_CONST, {.i64=MODE_LINE},  .flags=FLAGS, .unit="mode"},
> +        { "p2p",  "draw a point-to-point line for each sample",  0, AV_OPT_TYPE_CONST, {.i64=MODE_P2P},  .flags=FLAGS, .unit="mode"},
>      { "n",    "set how many samples to show in the same point", OFFSET(n), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, FLAGS },
>      { "rate", "set video rate", OFFSET(rate), AV_OPT_TYPE_VIDEO_RATE, {.str = "25"}, 0, 0, FLAGS },
>      { "r",    "set video rate", OFFSET(rate), AV_OPT_TYPE_VIDEO_RATE, {.str = "25"}, 0, 0, FLAGS },
> @@ -158,6 +171,122 @@ static int request_frame(AVFilterLink *outlink)
>      return ret;
>  }
>  
> +static void plot_point(AVFilterLink *inlink, int mf, int x, int y)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    ShowWavesContext *showwaves = ctx->priv;
> +    AVFrame *outpicref = showwaves->outpicref;
> +    int linesize = outpicref->linesize[0];

> +    *(outpicref->data[0] + x + y * linesize) += mf;

Bresenham's algorith is meant to avoid multiplications and divisions. If you
perform a multiplication to set the point, you are kind of missing the
point. The correct implementation would probably be to keep a single pointer
and offset instead of separate x and y. That also makes the implementation
simpler: instead of duplicating the loop for each quadrant, you only need to
init two increments: the one that is applied always and the one that is
applied when there is an overflow.

> +}
> +
> +static void plot_line(AVFilterLink *inlink, int mf, int x0, int y0, int x1, int y1)
> +{
> +    int octant;
> +    int i;
> +    /*
> +     *
> +     *
> +     *
> +     *        NW   N   NE
> +     *          \  |  /
> +     *           \ | /
> +     *            \|/
> +     *     W ------+------- E
> +     *            /|\
> +     *           / | \
> +     *          /  |  \
> +     *        SW   S   SE
> +     *
> +     *
> +     */
> +    if (x0 == x1 && y0 < y1)      octant = OCTANT_N;
> +    else if (x0 == x1 && y0 > y1) octant = OCTANT_S;
> +    else if (y0 == y1 && x0 < x1) octant = OCTANT_E;
> +    else if (y0 == y1 && x0 > x1) octant = OCTANT_W;
> +    else if (x0 < x1 && y0 < y1)  octant = OCTANT_NE;
> +    else if (x0 < x1 && y0 > y1)  octant = OCTANT_SE;
> +    else if (x0 > x1 && y0 > y1)  octant = OCTANT_SW;
> +    else if (x0 > x1 && y0 < y1)  octant = OCTANT_NW;

> +    switch (octant)
> +    {

The usual coding style in FFmpeg is to place the opening brace on the same
line as the block keyword.

> +        case OCTANT_S:
> +        case OCTANT_W:
> +        case OCTANT_SW:

> +        case OCTANT_NW:
> +        {

And not to create blocks for cases unless you need local variables.

> +            FFSWAP(int16_t, x0, x1);
> +            FFSWAP(int16_t, y0, y1);
> +            break;
> +        }
> +    }
> +    switch (octant)
> +    {
> +        case OCTANT_S:
> +        case OCTANT_N:
> +        {
> +            for (i = y0; i < y1; i++)
> +                plot_point(inlink, mf, x0, i);
> +            break;
> +        }
> +        case OCTANT_W:
> +        case OCTANT_E:
> +        {
> +            for (i = x0; i < x1; i++)
> +                plot_point(inlink, mf, i, y0);
> +            break;
> +        }
> +        /* http://en.wikipedia.org/wiki/Bresenham's_line_algorithm */
> +        case OCTANT_SW:
> +        case OCTANT_NE:
> +        {
> +            int dx = x1 - x0;
> +            int dy = y1 - y0;
> +            int d = 2 * dy - dx;
> +            int y = y0;
> +            for (i = x0 + 1; i <= x1; i++)
> +            {
> +                if (d > 0)
> +                {
> +                    y++;
> +                    plot_point(inlink, mf, i, y);
> +                    d = d + (2 * dy - 2 * dx);
> +                }
> +                else
> +                {
> +                    plot_point(inlink, mf, i, y);
> +                    d = d + 2 * dy;
> +                }
> +            }
> +            break;
> +        }
> +        /* http://en.wikipedia.org/wiki/Bresenham's_line_algorithm */
> +        case OCTANT_NW:
> +        case OCTANT_SE:
> +        {
> +            int dx = x1 - x0;
> +            int dy = y1 - y0;
> +            int d = 2 * dy - dx;
> +            int y = y0;
> +            for (i = x0 + 1; i <= x1; i++)
> +            {
> +                if (d > 0)
> +                {
> +                    y--;
> +                    plot_point(inlink, mf, i, y);
> +                    d = d + (2 * dy - 2 * dx);
> +                }
> +                else
> +                {
> +                    plot_point(inlink, mf, i, y);
> +                    d = d + 2 * dy;
> +                }
> +            }
> +            break;
> +        }
> +    }
> +}
> +
>  #define MAX_INT16 ((1<<15) -1)
>  
>  static int filter_frame(AVFilterLink *inlink, AVFrame *insamples)
> @@ -171,8 +300,11 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples)
>      int16_t *p = (int16_t *)insamples->data[0];
>      int nb_channels = inlink->channels;
>      int i, j, k, h, ret = 0;
> +    int pp_x0 = 0, pp_y0 = 0; /* current coords */
> +    int pp_x1 = 0, pp_y1 = 0; /* previous coords */
> +    const int pp_max_distance = showwaves->w < showwaves->h ? showwaves->w / 10 : showwaves->h / 10;
>      const int n = showwaves->n;
> -    const int x = 255 / (nb_channels * n); /* multiplication factor, pre-computed to avoid in-loop divisions */
> +    const int mf = 255 / (nb_channels * n); /* multiplication factor, pre-computed to avoid in-loop divisions */
>  
>      /* draw data in the buffer */
>      for (i = 0; i < nb_samples; i++) {
> @@ -196,7 +328,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples)
>              switch (showwaves->mode) {
>              case MODE_POINT:
>                  if (h >= 0 && h < outlink->h)
> -                    *(outpicref->data[0] + showwaves->buf_idx + h * linesize) += x;
> +                    *(outpicref->data[0] + showwaves->buf_idx + h * linesize) += mf;
>                  break;
>  
>              case MODE_LINE:
> @@ -204,7 +336,28 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples)
>                  int start = showwaves->h/2, end = av_clip(h, 0, outlink->h-1);
>                  if (start > end) FFSWAP(int16_t, start, end);
>                  for (k = start; k < end; k++)
> -                    *(outpicref->data[0] + showwaves->buf_idx + k * linesize) += x;
> +                    *(outpicref->data[0] + showwaves->buf_idx + k * linesize) += mf;
> +                break;
> +            }
> +            case MODE_P2P:
> +            {
> +                if (h >= 0 && h < outlink->h)
> +                {
> +                    pp_x0 = showwaves->buf_idx;
> +                    pp_y0 = h;
> +                    plot_point(inlink, mf, pp_x0, pp_y0);
> +                    if ((pp_x1 > 0 || pp_y1 > 0) && (pp_x0 != pp_x1 || pp_y0 != pp_y1))
> +                    {
> +                        /* Ignore points "too far" */
> +                        int pp_distance = pp_x0 > pp_x1 ? pp_x0 - pp_x1 : pp_x1 - pp_x0;
> +                        if (pp_distance < pp_max_distance)
> +                        {

> +                            plot_line(inlink, mf, pp_x0, pp_y0, pp_x1, pp_y1);

Unless I am mistaken, at this point, pp_x1 can only be equal to pp_x0+1, or
maybe just pp_x0. In either case, you do not need the full line-drawing
algorithm.

> +                        }
> +                    }
> +                    pp_x1 = pp_x0;
> +                    pp_y1 = pp_y0;
> +                }
>                  break;
>              }
>              }

Regards,

-- 
  Nicolas George


More information about the ffmpeg-devel mailing list