[FFmpeg-devel] [PATCH 4/4] examples: adding device_get_capabilities example

[Andreas Rheinhardt]

Diederick C. Niehorster:
> On Sat, Jun 5, 2021 at 8:51 AM Andreas Rheinhardt
> <andreas.rheinhardt at outlook.com> wrote:
>>
>> Diederick Niehorster:
>>> +
>>> +    // since there is no equivalent of avformat_alloc_output_context2 for an input context,
>>> +    // so we get this dirty code that users shouldn't have to write....
>>> +    fmt_ctx = avformat_alloc_context();
>>> +    fmt_ctx->url = av_strdup(dshow_input_name);
>>
>> Missing checks.
>>
>>> +    fmt_ctx->iformat = fmt;
>>> +    if (fmt_ctx->iformat->priv_data_size > 0) {
>>
>> priv_data_size is not part of the public API; so the code is indeed
>> dirty and so this example is unacceptable as-is. This might be a
>> scenario where AVFMT_FLAG_PRIV_OPT might have been useful, see
>> 2ff40b98ecbd9befadddc8fe665a391f99bfca32.
>>
>>> +        if (!(fmt_ctx->priv_data = av_mallocz(fmt_ctx->iformat->priv_data_size))) {
>>> +            ret = AVERROR(ENOMEM);
>>> +            goto end;
>>> +        }
>>> +        if (fmt_ctx->iformat->priv_class) {
>>> +            *(const AVClass**)fmt_ctx->priv_data = fmt_ctx->iformat->priv_class;
>>> +            av_opt_set_defaults(fmt_ctx->priv_data);
>>> +        }
>>> +    }
>>> +    // end dirty code
> 
> I have fixed the rest, but I believe it is not possible to fix the
> above problem with the current API? The code above shows the state i
> need the format context and attached input context to be in when
> calling av_opt_query_ranges. The device cannot be opened (that is
> read_header should not be called) before calling av_opt_query_ranges,
> as it is not possible to reliably query possible settings of the
> device once it is opened (without temporarily closing it anyway, which
> would be undesirable). One solution i see is to:
> 1. implement a avformat_alloc_input_context()
> 2. analogous to avformat_alloc_output_context2, add a few
> if-statements in avformat_open_input() to skip steps if they are
> already done on the passed-in AVFormatContext or its iformat.
> 

Have you looked at
2ff40b98ecbd9befadddc8fe665a391f99bfca32/0a071f7124beaf0929f772a8618ac1b6c17b0222?

> It would e.g. allow something along the lines of:
> AVFormatContext* fmt_ctx = NULL;
> avformat_alloc_input_context(&fmt_ctx, NULL, "dshow",
> "video=\"Integrated Webcam\"");
> // ...
> // query device capabilities, built up option dict for setting up
> device as wanted based on query results
> // ....
> avformat_open_input(&fmt_ctx,NULL,NULL,opts);
> // NB: filename and fmt inputs above are NULL as already set by
> avformat_alloc_input_context
> 
> Would such an API extension be considered?
> 
> NB: The constraint of my implementation of the AVDevice Capabilties
> API should also be documented. I propose, more generally: the
> avdevice_capabilities_create/av_opt_query_ranges API has the
> constraint that while it is always valid to query capabilities on an
> unopened avdevice (as provided by avformat_alloc_input_context), some
> devices may not allow querying capabilities once avformat_open_input()
> has been called on them. In that case, upon av_opt_query_ranges these
> devices will return AVERROR(EIO).
> 
Of course you can propose all sorts of API extensions; but don't be too
beholden to the current API. It has never been tested in practice and it
shows (e.g. it is built on top of the AVOptionRanges API which is quite
underdocumented (see the remark/gotcha about the AVOptionRange structs
not being individually allocated for your patch 3/4); and making each
query_range callback allocate an AVOptionRange unnecessarily ties
sizeof(AVOptionRange) to the ABI -- I don't get why this has been done).
Also what you are experiencing here shows that there probably never was
a working example.

Besides that, there are more weird things: Why is
AVDeviceCapabilitiesQuery a public struct and not an opaque one if it is
not to be used? Why is av_device_capabilities public if it should not be
used by a user?

- Andreas