From: "Diederick C. Niehorster" <dcnieho@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v5 10/13] avdevice/dshow: add media type info to get_device_list
Date: Mon, 20 Dec 2021 09:01:27 +0100
Message-ID: <CABcAi1gPRiHdQOABtW6HKW8r1d1ntPstBQeS89kweybsJe3rAQ@mail.gmail.com> (raw)
In-Reply-To: <AM7PR03MB66607C16E1F24D0C0249C4FE8F7B9@AM7PR03MB6660.eurprd03.prod.outlook.com>
On Mon, Dec 20, 2021 at 2:04 AM Andreas Rheinhardt
<andreas.rheinhardt@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.
Thanks!
Dee
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2021-12-20 8:01 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-19 19:21 [FFmpeg-devel] [PATCH v5 00/13] dshow enhancements Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 01/13] avdevice/dshow: prevent NULL access Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 02/13] avdevice/dshow: implement option to use device video timestamps Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 03/13] avdevice/dshow: add use_video_device_timestamps to docs Diederick Niehorster
2021-12-20 0:42 ` Andreas Rheinhardt
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 04/13] avdevice/dshow: query graph and sample time only once Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 05/13] avdevice/dshow: handle unknown sample time Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 06/13] avdevice/dshow: set no-seek flags Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 07/13] avdevice/dshow: implement get_device_list Diederick Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 08/13] avdevice/dshow: list_devices: show media type(s) per device Diederick Niehorster
2021-12-20 0:55 ` Andreas Rheinhardt
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 09/13] avdevice: add info about media types(s) to AVDeviceInfo Diederick Niehorster
2021-12-20 0:59 ` Andreas Rheinhardt
2021-12-20 9:54 ` Diederick C. Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 10/13] avdevice/dshow: add media type info to get_device_list Diederick Niehorster
2021-12-20 1:04 ` Andreas Rheinhardt
2021-12-20 8:01 ` Diederick C. Niehorster [this message]
2021-12-21 11:55 ` Diederick C. Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 11/13] fftools: provide media type info for devices Diederick Niehorster
2021-12-20 1:22 ` Andreas Rheinhardt
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 12/13] avdevice/dshow: discover source color range/space/etc Diederick Niehorster
2021-12-20 1:27 ` Andreas Rheinhardt
2021-12-20 8:10 ` Diederick C. Niehorster
2021-12-19 19:21 ` [FFmpeg-devel] [PATCH v5 13/13] avdevice/dshow: select format with extended color info Diederick Niehorster
2021-12-20 1:32 ` Andreas Rheinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABcAi1gPRiHdQOABtW6HKW8r1d1ntPstBQeS89kweybsJe3rAQ@mail.gmail.com \
--to=dcnieho@gmail.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git