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

[Thilo Borgmann]

Am 30.06.20 um 16:09 schrieb Ashutosh Pradhan:
> On Mon, Jun 29, 2020 at 6:53 PM Moritz Barsnick <barsnick at gmx.net> wrote:
> 
>> 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".
>>
> Will change to above.
> 
>>
>>>  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.
>>
> Yes.
> Windows doesn't have the recommended Fluid_R3 General Midi SoundFont file.
> So one has to enter the path every time.
> 
>>
>>> +++ 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.)
>>
> I didn't want to output an error incase the instrument did not match. I
> forgot to update 0 with the number of the default instrument. I will update
> that.

For that case, you might want to output a warning that the instrument could not be matched and the default instrument will be used 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".
>>

-Thilo