[FFmpeg-devel] [PATCH] lavfi: avectorscope filter

[Paul B Mahol]

On 5/10/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Monday 2013-04-29 13:17:58 +0000, Paul B Mahol encoded:
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  doc/filters.texi               |  44 +++++++
>>  libavfilter/Makefile           |   1 +
>>  libavfilter/allfilters.c       |   1 +
>>  libavfilter/avf_avectorscope.c | 262
>> +++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 308 insertions(+)
>>  create mode 100644 libavfilter/avf_avectorscope.c
>>
>> +

[...]

>> +    outlink->w = p->w;
>> +    outlink->h = p->h;
>> +    outlink->sample_aspect_ratio = (AVRational){1,1};
>> +    outlink->frame_rate = p->frame_rate;
>
>> +    p->outpicref = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> +    if (!p->outpicref)
>> +        return AVERROR(ENOMEM);
>
> This is potentially unsafe, since at this stage we are not sure that
> the filtergraph is already configured. Possibly safer: you ask the
> buffer lazily when you need it.

Makes sense, changed (as done in showwaves).

>
> How do we tackle this in other filters?

showspectrum does something similar, not sure its safe.

>
>> +    size = outlink->h * p->outpicref->linesize[0];
>> +    memset(p->outpicref->data[0] + size, 0, FFABS(size));
>> +
>> +    p->hw = p->w / 2;
>> +    p->hh = p->h / 2;
>> +
>> +    return 0;
>> +}
>> +
>> +static int filter_frame(AVFilterLink *inlink, AVFrame *insamples)
>> +{
>> +    AVFilterContext *ctx = inlink->dst;
>> +    AVFilterLink *outlink = ctx->outputs[0];
>> +    AudioVectorScopeContext *p = ctx->priv;
>> +
>> +    p->outpicref->pts = insamples->pts;
>> +
>> +    const int hw = p->hw;
>> +    const int hh = p->hh;
>> +    unsigned x, y;
>> +    int i;
>> +
>> +    fade(p);
>> +
>> +    switch (insamples->format) {
>> +    case AV_SAMPLE_FMT_S16:
>> +        for (i = 0; i < insamples->nb_samples; i++) {
>> +            int16_t *src = (int16_t *)insamples->data[0] + i * 2;
>> +
>
>> +            if (p->mode == LISSAJOUS) {
>> +                x = ((src[1] - src[0]) / (float)(UINT16_MAX) + 1) * hw;
>> +                y = (1.0 - (src[0] + src[1]) / (float)UINT16_MAX) * hh;
>
> what's +1 good for?

src1-src2 can give < 0 value, but we need > 0 value to get positive width,
otherwise output looks wrong.

>
>> +            } else {
>> +                x = (src[1] / (float)INT16_MAX + 1) * hw;
>> +                y = (src[0] / (float)INT16_MAX + 1) * hh;
>> +            }
>> +
>> +            draw_dot(p, x, y);
>> +        }
>> +        break;
>> +    case AV_SAMPLE_FMT_FLT:
>> +        for (i = 0; i < insamples->nb_samples; i++) {
>> +            float *src = (float *)insamples->data[0] + i * 2;
>> +
>> +            if (p->mode == LISSAJOUS) {
>> +                x = ((src[1] - src[0]) / 2 + 1) * hw;
>> +                y = (1.0 - (src[0] + src[1]) / 2) * hh;
>> +            } else {
>> +                x = (src[1] + 1) * hw;
>> +                y = (src[0] + 1) * hh;
>> +            }
>> +
>> +            draw_dot(p, x, y);
>> +        }
>> +        break;
>> +    }
>> +
>> +    av_frame_free(&insamples);
>> +
>> +    return ff_filter_frame(outlink, av_frame_clone(p->outpicref));
>> +}
>> +
>
> LGTM otherwise and very nice work.
> --
> FFmpeg = Faithless Formidable Majestic Portable Explosive Gymnast
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>