[FFmpeg-devel] [PATCH] avfilter: add ladspa wrapper filter

[Paul B Mahol]

On 9/22/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Saturday 2013-09-21 22:53:06 +0000, Paul B Mahol encoded:
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  configure                |   4 +
>>  doc/filters.texi         |  92 +++++++
>>  libavfilter/Makefile     |   1 +
>>  libavfilter/af_ladspa.c  | 666
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  libavfilter/allfilters.c |   1 +
>>  5 files changed, 764 insertions(+)
>>  create mode 100644 libavfilter/af_ladspa.c
>>

[...]

> Controls need to be defined using the following syntax:
> c0=@var{value0}|c1=@var{value1}|c2=@var{value2}|..., where
> @var{valuei} is the value set on the @var{i}-th control.
>
> If @option{controls} is set to @code{help}, all available controls and
> their valid ranges are printed.
>
> or something like that.

done.

>
>> +
>> + at item sample_rate, s
>> +Specify the sample rate, default to 44100. Only used if plugin have
>> +zero inputs.
>> +
>> + at item nb_samples, n
>> +Set the number of samples per channel per each output frame, default
>> +is 1024. Only used if plugin have zero inputs.
>> +
>> + at item duration, d
>> +Set the minimum duration of the sourced audio. See the function
>> + at code{av_parse_time()} for the accepted format, also check the "Time
>> duration"
>> +section in the ffmpeg-utils manual.
>
>> +Note that the resulting duration may be greater than
>> +the specified duration, as the generated audio is always cut at the
>> +end of a complete frame.
>
> nit: weird formatting

fixed

>
>> +If not specified, or the expressed duration is negative, the audio is
>> +supposed to be generated forever.
>> +Only used if plugin have zero inputs.
>> +
>> + at end table
>> +
>> + at subsection Examples
>> +
>> + at itemize
>> + at item
>> +List all available plugins within amp (LADSPA example plugin) library:
>> + at example
>> +ladspa=file=amp
>> + at end example

[...]

>> +    unsigned long nb_ports;
>> +
>> +    unsigned long nb_inputs;
>
>> +    unsigned long *ipmap;           /* map of input ports */
>
> map port number to input number?

other way around, improved description

>
>> +
>> +    unsigned long nb_inputcontrols;
>> +    unsigned long *icmap;           /* map of input controls */
>
>> +    LADSPA_Data *ictlv;             /* input cotrols values */
>
> typo

fixed

>
>> +
>> +    unsigned long nb_outputs;
>> +    unsigned long *opmap;           /* map of output ports */
>> +
>> +    unsigned long nb_outputcontrols;
>> +    unsigned long *ocmap;           /* map of output controls */
>> +    LADSPA_Data *octlv;             /* output controls values */
>> +
>> +    const LADSPA_Descriptor *desc;
>> +    int *ctl_needs_value;
>
>> +    int nb_handles;
>> +    LADSPA_Handle *handles;
>
> Can you explain the relation between handles, ports, and channels?

Here or in code?

>
>> +
>> +    int sample_rate;
>> +    int nb_samples;
>> +    int64_t pts;
>> +    int64_t duration;
>> +} LADSPAContext;
>> +

[...]

>> +{
>> +    char *path = av_asprintf("%s/%s.so", dir, soname);
>> +    if (!path)
>> +        return NULL;
>> +    return dlopen(path, RTLD_NOW);
>
> leak on path

Thanks. Fixed.

>
>> +}
>> +
>> +static av_cold int init(AVFilterContext *ctx)
>> +{
>> +    LADSPAContext *s = ctx->priv;
>> +    LADSPA_Descriptor_Function descriptor_fn;
>> +    const LADSPA_Descriptor *desc;
>> +    LADSPA_PortDescriptor pd;
>> +    AVFilterPad pad = { NULL };
>> +    char *p, *arg, *saveptr = NULL;
>> +    int i;
>> +
>> +    if (!s->dl_name) {
>> +        av_log(ctx, AV_LOG_ERROR, "No plugin name provided\n");
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>> +    if (s->dl_name[0] == '/' || s->dl_name[0] == '.') {
>> +        // argument is a path
>> +        s->dl_handle = dlopen(s->dl_name, RTLD_NOW|RTLD_LOCAL);
>
> Any reason to use RTLD_LOCAL here (and not in try_load)?

fixed

>
> You could refactor and use try_load here as well.
>
>> +    } else {
>> +        // argument is a shared object name
>> +        char *paths = av_strdup(getenv("LADSPA_PATH"));
>> +        const char *separator = ":";
>> +

[...]

>> +    }
>> +
>
>> +    if (s->nb_inputs == 1 && s->nb_outputs == 1) {
>
>> +        // We will instantiate multiple instances, one over each channel
>
> confusing: instances of what?

LADSPA_Handle

>
>> +        layouts = ff_all_channel_layouts();
>> +        if (!layouts)
>> +            return AVERROR(ENOMEM);
>> +        ff_set_common_channel_layouts(ctx, layouts);
>> +    } else {
>> +        if (s->nb_inputs >= 1) {
>> +            AVFilterLink *inlink = ctx->inputs[0];
>> +            int64_t inlayout = FF_COUNT2LAYOUT(s->nb_inputs);
>> +
>> +            layouts = NULL;
>> +            ff_add_channel_layout(&layouts, inlayout);
>> +            ff_channel_layouts_ref(layouts,
>> &inlink->out_channel_layouts);
>> +        }
>
> So you have N inputs, you create N inputs, each one with N channels?

each input/output port in ladspa maps to single input/output channel.

input controls ports are our AVOptions.

output controls ports are something like our metadata/logs.

>
>> +
>> +        if (s->nb_outputs >= 1) {
>> +            AVFilterLink *outlink = ctx->outputs[0];
>> +            int64_t outlayout = FF_COUNT2LAYOUT(s->nb_outputs);
>> +
>> +            layouts = NULL;
>> +            ff_add_channel_layout(&layouts, outlayout);
>> +            ff_channel_layouts_ref(layouts,
>> &outlink->in_channel_layouts);
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
> [...]

I'm very happy with quality this patch reached so I will push it.