[FFmpeg-devel] [PATCH 3/4] avdevice/dshow: implement capabilities API

[Diederick C. Niehorster]

On Sat, Jun 5, 2021 at 2:25 PM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
> Diederick Niehorster:
> > This implements avdevice_capabilities_create and avdevice_capabilities_free for the dshow device.> > +
> > +            if (ranges) {
> > +                if (query_type == CAP_QUERY_FRAME_SIZE) {
> > +                    for (int j = 0; j < 3; j++) {
> > +                        range = av_mallocz(sizeof(AVOptionRange));
>
> It seems that you are allocating individual AVOptionRange structures.
> This is bad, because that is not how av_opt_freep_ranges() treats them:
> They are supposed to be pointers into one big array of AVOptionRange
> structures.

av_opt_freep_ranges() does this (NB: ranges->range is an AVOptionRange **):
for (i = 0; i < ranges->nb_ranges * ranges->nb_components; i++) {
        AVOptionRange *range = ranges->range[i];
        if (range) {
            av_freep(&range->str);
            av_freep(&ranges->range[i]); // <--
        }
    }
    av_freep(&ranges->range);

Note line highlighted by <--:
it frees each individual element, one at a time, and only after that
deletes the whole array. So i think it is correct that i am allocating
them one at a time? Am i misreading/-understanding this?

> > +    const AVFormatContext *avctx = caps->device_context;
>
> avctx is only used for AVCodecContexts by convention.

the dshow device (almost) consistently uses avctx as name for the
AVFormatContext. Should i change that everywhere (what is the
convention name for a AVFormatContext?), or is it ok if i remain
consistent with that here?

> > +    if (ctx->device_name[0]) {
> > +        av_log(avctx, AV_LOG_ERROR, "You cannot query device capabilities on an opened device\n");
>
> Is this supposed to be a limitation of the capabilities API in general
> or only of the implementation here in for dshow?

See other mails: the bit of documentation in avdevice.h (lines
334--402) suggest the capabilities API is supposed to be used on an
unopened device. It would make sense: probe the device for what it can
do, decide what settings you want to use, then open device). In case
of the dshow device, I think i cannot avoid potential false positives
(entries in the query output that shouldn't be there) if the device is
already opened. I'll experiment, perhaps this is only a theoretical
worry.

Thanks!
Dee