[FFmpeg-devel] [PATCH 1/2] lavfi: add astreamsync audio filter.

[Stefano Sabatini]

On date Friday 2011-12-23 14:17:13 +0100, Nicolas George encoded:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  Changelog                    |    1 +
>  doc/filters.texi             |   29 ++++++
>  libavfilter/Makefile         |    1 +
>  libavfilter/af_astreamsync.c |  204 ++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/allfilters.c     |    1 +
>  5 files changed, 236 insertions(+), 0 deletions(-)
>  create mode 100644 libavfilter/af_astreamsync.c
[...]
> diff --git a/libavfilter/af_astreamsync.c b/libavfilter/af_astreamsync.c
> new file mode 100644
> index 0000000..66a8b63
> --- /dev/null
> +++ b/libavfilter/af_astreamsync.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (c) 2011 Nicolas George <nicolas.george at normalesup.org>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Stream (de)synchronization filter
> + */
> +

> +#include <stdlib.h>
> +#include "libavcodec/avcodec.h"

not needed?

> +#include "libavutil/avstring.h"
> +#include "libavutil/eval.h"
> +#include "libswresample/swresample.h" // only for SWR_CH_MAX
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +#define QUEUE_SIZE 16
> +
> +static const char * const var_names[] = {
> +    "b1", "b2",
> +    "s1", "s2",
> +    "t1", "t2",
> +    NULL
> +};
> +
> +enum var_name {
> +    VAR_B1, VAR_B2,
> +    VAR_S1, VAR_S2,
> +    VAR_T1, VAR_T2,
> +    VAR_NB
> +};
> +
> +struct astreamsync_context {
> +    AVExpr *expr;
> +    double var_values[VAR_NB];
> +    struct {
> +        AVFilterBufferRef *buf[QUEUE_SIZE];
> +        unsigned tail, nb;

please provide some comment, or it will harder to read/understand this
code

> +    } queue[2];
> +    int req[2];
> +    int next_out;

> +    int eof;

maybe note that this is a bitmap, one bit for each input
pad/link (maybe using an array instead may help readability)

> +};

Nit++: AStreamSyncContext or ASyncContext or whatever for consistency
sake.

> +
> +static const char *default_expr = "t1-t2";
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args0, void *opaque)
> +{
> +    struct astreamsync_context *as = ctx->priv;
> +    const char *expr = args0 ? args0 : default_expr;
> +    int r, i;
> +
> +    r = av_expr_parse(&as->expr, expr, var_names,
> +                      NULL, NULL, NULL, NULL, 0, ctx);
> +    if (r < 0) {
> +        av_log(ctx, AV_LOG_ERROR, "Error in expression \"%s\"\n", expr);
> +        return r;
> +    }

> +    for (i = 0; i < 42; i++)
> +        av_expr_eval(as->expr, as->var_values, NULL); /* exercize prng */

uh?

> +    return 0;
> +}
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    int i;
> +    AVFilterFormats *formats;
> +
> +    for (i = 0; i < 2; i++) {
> +        formats = ctx->inputs[i]->in_formats;
> +        avfilter_formats_ref(formats, &ctx->inputs[i]->out_formats);
> +        avfilter_formats_ref(formats, &ctx->outputs[i]->in_formats);
> +        formats = ctx->inputs[i]->in_packing;
> +        avfilter_formats_ref(formats, &ctx->inputs[i]->out_packing);
> +        avfilter_formats_ref(formats, &ctx->outputs[i]->in_packing);
> +        formats = ctx->inputs[i]->in_chlayouts;
> +        avfilter_formats_ref(formats, &ctx->inputs[i]->out_chlayouts);
> +        avfilter_formats_ref(formats, &ctx->outputs[i]->in_chlayouts);
> +    }
> +    return 0;
> +}
> +
> +static int config_output(AVFilterLink *outlink)
> +{
> +    return 0;
> +}
> +

> +static void send_out(AVFilterContext *ctx, int id)

nit: qid for making it more clear that id refers a queue (pad_id ->
pid or link_id -> lid would be fine as well)

> +{
> +    struct astreamsync_context *as = ctx->priv;
> +    AVFilterBufferRef *buf = as->queue[id].buf[as->queue[id].tail];

nit:
*queue = as->queue[id];
should ease readability:

as->queue[id].buf[as->queue[id].tail];
->
queue->buf[queue->tail];

> +
> +    as->queue[id].buf[as->queue[id].tail] = NULL;
> +    as->var_values[VAR_B1 + id]++;
> +    as->var_values[VAR_S1 + id] += buf->audio->nb_samples;
> +    if (buf->pts != AV_NOPTS_VALUE)
> +        as->var_values[VAR_T1 + id] = av_q2d(ctx->inputs[id]->time_base) *
> +                                      buf->pts;
> +    as->var_values[VAR_T1 + id] += buf->audio->nb_samples /
> +                                   (double)ctx->inputs[id]->sample_rate;
> +    avfilter_filter_samples(ctx->outputs[id], buf);
> +    as->queue[id].nb--;
> +    as->queue[id].tail = (as->queue[id].tail + 1) % QUEUE_SIZE;
> +    if (as->req[id])
> +        as->req[id]--;
> +}
> +
> +static void send_next(AVFilterContext *ctx)
> +{
> +    struct astreamsync_context *as = ctx->priv;
> +    int i;
> +
> +    while (1) {
> +        if (!as->queue[as->next_out].nb)
> +            break;
> +        send_out(ctx, as->next_out);
> +        if (!as->eof)
> +            as->next_out = av_expr_eval(as->expr, as->var_values, NULL) >= 0;
> +    }
> +    for (i = 0; i < 2; i++)
> +        if (as->queue[i].nb == QUEUE_SIZE)
> +            send_out(ctx, i);
> +}
> +
> +static int request_frame(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    struct astreamsync_context *as = ctx->priv;
> +    int id = outlink == ctx->outputs[1];
> +
> +    as->req[id]++;
> +    while (as->req[id] && !(as->eof & (1 << id))) {
> +        if (as->queue[as->next_out].nb) {
> +            send_next(ctx);
> +        } else {
> +            as->eof |= 1 << as->next_out;
> +            avfilter_request_frame(ctx->inputs[as->next_out]);
> +            if (as->eof & (1 << as->next_out))
> +                as->next_out = !as->next_out;
> +        }
> +    }
> +    return 0;
> +}
> +
> +static void filter_samples(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    struct astreamsync_context *as = ctx->priv;
> +    int id = inlink == ctx->inputs[1];
> +

> +    as->queue[id].buf[(as->queue[id].tail + as->queue[id].nb++) % QUEUE_SIZE] =
> +        insamples;

circular buffer overflow? shouldn't you check that the buffer is not
already filled (and in that case maybe release memory)?

> +    as->eof &= ~(1 << id);
> +    send_next(ctx);
> +}
> +
> +AVFilter avfilter_af_astreamsync = {
> +    .name          = "astreamsync",

> +    .description   = NULL_IF_CONFIG_SMALL("Copy two streams of audio data "
> +                                          "in a configurable order"),

Nit: missing final dot.
-- 
FFmpeg = Fabulous Fiendish Magnificient Portable Elfic Gadget