diff mbox series

[FFmpeg-devel,v2,1/5] libdc1394: Enable listing sources

Message ID 20201015213039.1019624-2-cyrozap@gmail.com
State New
Headers show
Series libdc1394 enhancements | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Forest Crossman Oct. 15, 2020, 9:30 p.m. UTC
IIDC camera sources can now be listed by using the following command:

    ffmpeg -sources libdc1394

The basic structure of this function was borrowed from
libavdevice/alsa.c:ff_alsa_get_device_list.

Signed-off-by: Forest Crossman <cyrozap@gmail.com>
---
 libavdevice/libdc1394.c | 73 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

Comments

Marton Balint Oct. 28, 2020, 11:13 p.m. UTC | #1
On Thu, 15 Oct 2020, Forest Crossman wrote:

> IIDC camera sources can now be listed by using the following command:
>
>    ffmpeg -sources libdc1394
>
> The basic structure of this function was borrowed from
> libavdevice/alsa.c:ff_alsa_get_device_list.
>
> Signed-off-by: Forest Crossman <cyrozap@gmail.com>
> ---
> libavdevice/libdc1394.c | 73 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/libavdevice/libdc1394.c b/libavdevice/libdc1394.c
> index 003335fdd8..ec74cea87a 100644
> --- a/libavdevice/libdc1394.c
> +++ b/libavdevice/libdc1394.c
> @@ -22,6 +22,8 @@
> 
> #include <dc1394/dc1394.h>
> 
> +#include "avdevice.h"
> +
> #include "libavutil/imgutils.h"
> #include "libavutil/internal.h"
> #include "libavutil/log.h"
> @@ -288,6 +290,76 @@ static int dc1394_close(AVFormatContext * context)
>     return 0;
> }
> 
> +static int dc1394_get_device_list(AVFormatContext *c, AVDeviceInfoList *device_list)
> +{
> +    int ret = 0;
> +    uint32_t i;
> +    dc1394_t *d;
> +    dc1394camera_t *camera;
> +    dc1394camera_list_t *list;
> +    AVDeviceInfo *new_device = NULL;
> +
> +    d = dc1394_new();

Can't this return NULL on failure?

> +    if (dc1394_camera_enumerate(d, &list) != DC1394_SUCCESS || !list) {
> +        av_log(c, AV_LOG_ERROR, "Unable to look for an IIDC camera.\n");
> +        ret = -1;

AVERROR_EXTERNAL

> +        goto out;
> +    }
> +
> +    device_list->nb_devices = 0;
> +    device_list->default_device = -1;
> +
> +    if (list->num > 0) {
> +        device_list->default_device = 0;
> +    }

You can omit the braces.

> +
> +    for (i = 0; i < list->num; i++) {
> +        new_device = av_mallocz(sizeof(AVDeviceInfo));
> +        if (!new_device) {
> +            ret = AVERROR(ENOMEM);
> +            break;
> +        }
> +
> +        camera = dc1394_camera_new_unit(d, list->ids[i].guid, list->ids[i].unit);
> +        if (!camera) {
> +            av_log(c, AV_LOG_ERROR, "Unable to open camera 0x%016"PRIx64":%"PRIu16"\n",
> +                   list->ids[i].guid, list->ids[i].unit);
> +            ret = -1;

Does it makes sense to continue the loop on this failure? If not, then use 
AVERROR_EXTERNAL error code.

> +            break;
> +        }
> +
> +        new_device->device_name = av_asprintf("0x%016"PRIx64":%"PRIu16,
> +                                              list->ids[i].guid, list->ids[i].unit);
> +        new_device->device_description = av_asprintf("%s %s",
> +                                                     camera->vendor, camera->model);
> +        dc1394_camera_free(camera);
> +
> +        if (!new_device->device_description || !new_device->device_name) {
> +            ret = AVERROR(ENOMEM);
> +            break;
> +        }
> +
> +        if ((ret = av_dynarray_add_nofree(&device_list->devices,
> +                                          &device_list->nb_devices, new_device)) < 0) {
> +            break;
> +        }
> +
> +        new_device = NULL;
> +    }
> +    if (new_device) {
> +        av_free(new_device->device_description);
> +        av_free(new_device->device_name);

This can potentially free an already allocated device name/description if 
the allocation of new_device for the next item fails. Alsa.c uses local 
variables to avoid this issue...

Regards,
Marton

> +        av_free(new_device);
> +    }
> +
> +    /* Freeing list of cameras */
> +    dc1394_camera_free_list(list);
> +
> +out:
> +    dc1394_free(d);
> +    return ret;
> +}
> +
> AVInputFormat ff_libdc1394_demuxer = {
>     .name           = "libdc1394",
>     .long_name      = NULL_IF_CONFIG_SMALL("dc1394 v.2 A/V grab"),
> @@ -295,6 +367,7 @@ AVInputFormat ff_libdc1394_demuxer = {
>     .read_header    = dc1394_read_header,
>     .read_packet    = dc1394_read_packet,
>     .read_close     = dc1394_close,
> +    .get_device_list = dc1394_get_device_list,
>     .flags          = AVFMT_NOFILE,
>     .priv_class     = &libdc1394_class,
> };
> -- 
> 2.20.1
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavdevice/libdc1394.c b/libavdevice/libdc1394.c
index 003335fdd8..ec74cea87a 100644
--- a/libavdevice/libdc1394.c
+++ b/libavdevice/libdc1394.c
@@ -22,6 +22,8 @@ 
 
 #include <dc1394/dc1394.h>
 
+#include "avdevice.h"
+
 #include "libavutil/imgutils.h"
 #include "libavutil/internal.h"
 #include "libavutil/log.h"
@@ -288,6 +290,76 @@  static int dc1394_close(AVFormatContext * context)
     return 0;
 }
 
+static int dc1394_get_device_list(AVFormatContext *c, AVDeviceInfoList *device_list)
+{
+    int ret = 0;
+    uint32_t i;
+    dc1394_t *d;
+    dc1394camera_t *camera;
+    dc1394camera_list_t *list;
+    AVDeviceInfo *new_device = NULL;
+
+    d = dc1394_new();
+    if (dc1394_camera_enumerate(d, &list) != DC1394_SUCCESS || !list) {
+        av_log(c, AV_LOG_ERROR, "Unable to look for an IIDC camera.\n");
+        ret = -1;
+        goto out;
+    }
+
+    device_list->nb_devices = 0;
+    device_list->default_device = -1;
+
+    if (list->num > 0) {
+        device_list->default_device = 0;
+    }
+
+    for (i = 0; i < list->num; i++) {
+        new_device = av_mallocz(sizeof(AVDeviceInfo));
+        if (!new_device) {
+            ret = AVERROR(ENOMEM);
+            break;
+        }
+
+        camera = dc1394_camera_new_unit(d, list->ids[i].guid, list->ids[i].unit);
+        if (!camera) {
+            av_log(c, AV_LOG_ERROR, "Unable to open camera 0x%016"PRIx64":%"PRIu16"\n",
+                   list->ids[i].guid, list->ids[i].unit);
+            ret = -1;
+            break;
+        }
+
+        new_device->device_name = av_asprintf("0x%016"PRIx64":%"PRIu16,
+                                              list->ids[i].guid, list->ids[i].unit);
+        new_device->device_description = av_asprintf("%s %s",
+                                                     camera->vendor, camera->model);
+        dc1394_camera_free(camera);
+
+        if (!new_device->device_description || !new_device->device_name) {
+            ret = AVERROR(ENOMEM);
+            break;
+        }
+
+        if ((ret = av_dynarray_add_nofree(&device_list->devices,
+                                          &device_list->nb_devices, new_device)) < 0) {
+            break;
+        }
+
+        new_device = NULL;
+    }
+    if (new_device) {
+        av_free(new_device->device_description);
+        av_free(new_device->device_name);
+        av_free(new_device);
+    }
+
+    /* Freeing list of cameras */
+    dc1394_camera_free_list(list);
+
+out:
+    dc1394_free(d);
+    return ret;
+}
+
 AVInputFormat ff_libdc1394_demuxer = {
     .name           = "libdc1394",
     .long_name      = NULL_IF_CONFIG_SMALL("dc1394 v.2 A/V grab"),
@@ -295,6 +367,7 @@  AVInputFormat ff_libdc1394_demuxer = {
     .read_header    = dc1394_read_header,
     .read_packet    = dc1394_read_packet,
     .read_close     = dc1394_close,
+    .get_device_list = dc1394_get_device_list,
     .flags          = AVFMT_NOFILE,
     .priv_class     = &libdc1394_class,
 };