[FFmpeg-devel] [PATCH v5 10/13] avdevice/dshow: add media type info to get_device_list

[Diederick C. Niehorster]

Hi Andreas,

On Mon, Dec 20, 2021 at 9:01 AM Diederick C. Niehorster
<dcnieho at gmail.com> wrote:
>
> On Mon, Dec 20, 2021 at 2:04 AM Andreas Rheinhardt
> <andreas.rheinhardt at outlook.com> wrote:
> >
> > Diederick Niehorster:
> > >                  // store to device_list output
> > >                  if (av_reallocp_array(&(*device_list)->devices,
> > >                                       (*device_list)->nb_devices + 1,
> > > @@ -412,6 +417,8 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum,
> > >                  av_freep(&device->device_name);
> > >              if (device->device_name)
> > >                  av_freep(&device->device_description);
> > > +            if (device->media_types)
> > > +                av_freep(&device->media_types);
> >
> > You are duplicating freeing code here: You have code to free media_types
> > both before it was put into device and after; you can avoid the latter
> > by only attaching media_types to device when nothing else can fail.
>
> I could indeed avoid adding av_freep(&device->media_types) when only
> attaching media_types to the device once it made it into the device
> list array (so doing
> (*device_list)->devices[(*device_list)->nb_devices]->media_types =
> media_types). But that makes the code harder to read (asymmetric,
> everything else is attached to the device before it goes into the
> list) and the missing free while the other device members are freed
> looks like a code smell. I think this is easier to read and less error
> prone for future patches when done as currently.

Nevermind, with some reordering, i see i can do as you suggest. Will do.

Cheers,
Dee