[FFmpeg-devel] [PATCH][GSOC] avfilter: add atone filter

[Moritz Barsnick]

Hi Marshall,

On Sat, Mar 14, 2020 at 15:58:32 +0530, Marshall Murmu wrote:

> +    {"soundfont",   "location of soundfont",                     OFFSET(soundfont),   AV_OPT_TYPE_STRING,   {.str=NULL},     CHAR_MIN, CHAR_MAX,  FLAGS},

I don't think we define ranges for AV_OPT_TYPE_STRING.

> +    if(fluidsynth->settings == NULL) {

As Paul mentioned, whitespace style (throughout the complete source).

Also, "if (!fluidsynth->settings)" is the preferred style.

> +        av_log(ctx, AV_LOG_ERROR, "Failed to create fluidsynth settings\n");
> +        return AVERROR_UNKNOWN;

I'm not sure this is the correct error. If the library can only fail
due to lack of memory, this should be AVERROR(ENOMEM). Else failing
external library calls lead to AVERROR_EXTERNAL.

> +    if(fluidsynth->soundfont_id < 0) {
> +        av_log(ctx, AV_LOG_ERROR, "Failed to load soundfont\n");
> +    }

Shouldn't this also lead to an error being returned?
OTOH, if it isn't fatal, the log message should be a warning.

> +    return 0;
> +}

Cheers,
Moritz