[FFmpeg-devel] [PATCH]libavfilter/asrc_atone.c : generate algorithmic music

[Moritz Barsnick]

On Thu, Jun 25, 2020 at 21:15:06 +0530, Ashutosh Pradhan wrote:
> Generate algorithmic riff music. Changed to activate api.

This commit doesn't change to activate API, as it introduces this
filter. So that sentence doesn't belong here.

>  - Real War KVAG muxer
> -
> +- atone filter

Don't remove the empty line.

>    --enable-libflite        enable flite (voice synthesis) support via libflite [no]
> +  --enable-libfluidsynth   enable libfluidsynth support for fluidsynth [no]

"for fluidsynth"?. Perhaps "for atone source".

>  atempo_filter_select="rdft"
> +atone_filter_deps="libfluidsynth"

> + at item sfont
> +Specify the location of soundfont file. Default value is
> +"/usr/share/sounds/sf2/FluidR3_GM.sf2"(for linux).

For Linux - and elsewhere?
It will be the same path on all Unixes, but meaningless under Windows,
I guess.

> +++ b/libavfilter/Makefile
> @@ -152,6 +152,7 @@ OBJS-$(CONFIG_FLITE_FILTER)                  += asrc_flite.o
>  OBJS-$(CONFIG_HILBERT_FILTER)                += asrc_hilbert.o
>  OBJS-$(CONFIG_SINC_FILTER)                   += asrc_sinc.o
>  OBJS-$(CONFIG_SINE_FILTER)                   += asrc_sine.o
> +OBJS-$(CONFIG_ATONE_FILTER)                  += asrc_atone.o

I believe the audio sources are meant to be sorted alphabetically (by
CONFIG_*).

> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -146,6 +146,7 @@ extern AVFilter ff_asrc_flite;
>  extern AVFilter ff_asrc_hilbert;
>  extern AVFilter ff_asrc_sinc;
>  extern AVFilter ff_asrc_sine;
> +extern AVFilter ff_asrc_atone;

Same here.

> +#include <float.h>

> +    s->beat_dur = 60000/s->beats_pm;
> +    /*get change interval in frames/sec*/
> +    s->changerate = (4*s->beat_dur)*s->sample_rate/s->nb_samples;
> +    if (s->changerate<1.0)
> +        s->changerate = 1.0;

Please leave single spaces around the operators such as '/', '*', '<'.

> +    s->numriffs = sizeof(riff)/(NPR* sizeof(int));

Same here.

> +    for (int i = 0; i < s->numriffs*NPR ; i++)

And here.

> +    if (s->framecount == INT_MAX)
> +       s->framecount = 0;

Indentation.

> +    for (int i = 0; i < sizeof(GM_instrument_list)/sizeof(GM_instrument_list[0]); i++)

Use macro FF_ARRAY_ELEMS

> +        if (strcmp(GM_instrument_list[i], instrument) == 0)
> +            return i;
> +
> +    return 0;
> +}

If this doesn't match any string, it returns 0. It also returns 0 if it
matches "Acoustic-Grand". Is that correct? (In find_percussion_track(),
you return i instead.)

> +static void schedule_noteon(int chan, short key, unsigned int ticks, int velocity, AtoneContext *s)
> +{
> +   fluid_event_t *ev = new_fluid_event();

Indentation.

> +    unsigned rand = av_lfg_get(&s->r)/2;
[...]
> +        riff = rand%s->numriffs;

Spaces around operators, please.

> +    if (3*i < numbars)
> +        return (100 - (90*i)/numbars);
> +    else if (3*i > 2*numbars)
> +        return (40 + (90*i)/numbars);

Same here.

> +/*Determine the pattern, tempo (to paly as 8th, 16th or 32nd notes) and add the riffs to sequencer

"play"

> +Refernce: http://peterlangston.com/Papers/amc.pdf */

"Reference"

> +    AVFrame *frame;
> +    int  nb_samples;
          ^^ two spaces, drop one.

> +    ret = ff_set_common_formats (ctx, formats);
                                  ^ drop the space

> +typedef struct
> +{

Bracket on the same line.

> +Refernce: http://peterlangston.com/Papers/amc.pdf */

"Reference".


Regards,
Moritz