Calling av_opt_set_array() for AVChannelLayout and AVSampleRates in aformat filter

[ed-gh]

Hi Paul,
thank you for sharing the code. I've been tinkering with it for a couple of days now and eventually hit a problem I couldn't solve without a workaround:

It currently seems the aformat filter is the only one using AVOptions arrays (I made a small function to iterate over all filters and dump their AVOption info).

When building a filtergraph (e.g. anoisesrc -> aformat -> abuffersink) and trying to set the AVOptions for aformat (AVSampleFormat(s), sample rate(s) [1], AVChannelLayout(s)), the log receives an error message from the call to av_opt_set_array() called from draw_options(): 'Cannot set array element [n] for parameter sample_fmts: value [N] is out of rage [0-0].'
Turns out this is because the filter definition for aformat's AVOptions doesn't specify a .max for sample_fmts and channel_layouts.

Case in point:
Albeit this can circumvented by serializing and concatening the array to a string with '|' separators and then calling av_opt_set(), I'm wondering if there is a way to call av_opt_set_array() directly without receiving an error, or if this requires a fix in aformat.c 's AVOptions definition.

Similarly for AVChannelLayout, which currently isn't handled and after adding the respective code in my local repo, the same workaround (serialize and call av_opt_set()) did the job there too, but it seems a little kludgy.

[1] (Changing sample_rates doesn't cause an error message because .min and .max are defined. Mentioning this just for completeness.)

[ed-gh]

for illustration:

// runs into error:
case AV_OPT_TYPE_SAMPLE_FMT:
{
    int *formats = (int *)av_calloc(nb_elems, sizeof(*formats));
    bool new_value = false;

    // ...
    // set formats, new_value is set to true here
    // ...

    if (new_value)
        av_opt_set_array(av_class, opt->name, AV_OPT_ARRAY_REPLACE, 0, nb_elems, type, formats);
}

// workaround
case AV_OPT_TYPE_SAMPLE_FMT:
{
    int *formats = (int *)av_calloc(nb_elems, sizeof(*formats));
    bool new_value = false;

    // ...
    // set formats, new_value is set to true here
    // ...

    if (new_value)
    {
        std::stringstream expr;

        for (int64_t i = 0; i < static_cast<int64_t>(nb_elems); i++)
             expr << av_get_sample_fmt_name((AVSampleFormat)formats[i]) << "|";
            
        auto s_expr = expr.str();

        if (!s_expr.empty() && s_expr.back() == '|')
            s_expr.pop_back();

        av_opt_set(av_class, opt->name, s_expr.c_str(), AV_OPT_SEARCH_CHILDREN);
    }
}

[richardpl]

The librempeg build of lavfi-preview works fine here, except I need to add handling for channel layout array option.
The ffmpeg state of code is very buggy lately, so I do not recommend to use it at all.
Also you do not need to manually insert (a)format filters. Will check if lavfi-preview binaries that are using FFmpeg code can be improved in this case.

[ed-gh]

Yes, I can see you added the fix in librempeg about a year ago in commit 52180b0d86e3355d185c5259a731d0600655cfad.

I wasn't aware of librempeg until now. Thanks for the pointer!

Also, I understand there is little benefit in manually inserting the aformat filter, it was just an exercise to check the code branch reading from and writing to the AVOption arrays.

[richardpl]

There are more filters in librempeg that use avoptions array, previously such filters used string type options.
Which is mostly unfriendly for GUI like apps where it makes more sense to select between options if possible.

[ed-gh]

I will be looking into this, thank you!

[richardpl]

Unfortunately, I can not fix this on my side. complain to FFmpeg developers about ffmpeg aformat filter not signaling proper range for sample formats and rates. Interestingly FFmpeg abuffersink filter have .max set, and buffersrc have both .min and .max set.