[FFmpeg-devel] avdevice/decklink: add support for selecting devices based on their unique ID

Submitted by Marton Balint on Sept. 20, 2018, 10:05 p.m.

Details

Message ID 20180920220531.1044-1-cus@passwd.hu
State New
Headers show

Commit Message

Marton Balint Sept. 20, 2018, 10:05 p.m.
Also bump the API version requirement to 10.9.5, because on olders versions
there were some reports of crashes using the undocumented, yet available
BMDDeckLinkDeviceHandle.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 configure                       |  2 +-
 doc/indevs.texi                 |  3 ++-
 doc/outdevs.texi                |  3 ++-
 libavdevice/decklink_common.cpp | 59 ++++++++++++++++++++++++++++-------------
 libavdevice/decklink_common.h   |  1 -
 5 files changed, 45 insertions(+), 23 deletions(-)

Comments

kjeyapal@akamai.com Sept. 21, 2018, 4:48 a.m.
On 9/21/18 3:35 AM, Marton Balint wrote:
> Also bump the API version requirement to 10.9.5, because on olders versions

> there were some reports of crashes using the undocumented, yet available

> BMDDeckLinkDeviceHandle.

If we are using an undocumented API feature, then there is a higher risk of this feature not working in some future versions of DeckLink drivers as well.
I would suggest using this undocumented API call under an 'if' condition, which is controlled by a command line option for selecting based on unique ID. In that way setups which prefers stability can choose to not use this feature.
>

> Signed-off-by: Marton Balint <cus@passwd.hu>

> ---

>  configure                       |  2 +-

>  doc/indevs.texi                 |  3 ++-

>  doc/outdevs.texi                |  3 ++-

>  libavdevice/decklink_common.cpp | 59 ++++++++++++++++++++++++++++-------------

>  libavdevice/decklink_common.h   |  1 -

>  5 files changed, 45 insertions(+), 23 deletions(-)

>

> diff --git a/configure b/configure

> index 25120337de..64a519a8b3 100755

> --- a/configure

> +++ b/configure

> @@ -6043,7 +6043,7 @@ done

>  enabled cuda_sdk          && require cuda_sdk cuda.h cuCtxCreate -lcuda

>  enabled chromaprint       && require chromaprint chromaprint.h chromaprint_get_version -lchromaprint

>  enabled decklink          && { require_headers DeckLinkAPI.h &&

> -                               { test_cpp_condition DeckLinkAPIVersion.h "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a060100" || die "ERROR: Decklink API version must be >= 10.6.1."; } }

> +                               { test_cpp_condition DeckLinkAPIVersion.h "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a090500" || die "ERROR: Decklink API version must be >= 10.9.5."; } }

>  enabled libndi_newtek     && require_headers Processing.NDI.Lib.h

>  enabled frei0r            && require_headers frei0r.h

>  enabled gmp               && require gmp gmp.h mpz_export -lgmp

> diff --git a/doc/indevs.texi b/doc/indevs.texi

> index 5d4c02c597..ed2784be9f 100644

> --- a/doc/indevs.texi

> +++ b/doc/indevs.texi

> @@ -267,7 +267,8 @@ audio track.

>  

>  @item list_devices

>  If set to @option{true}, print a list of devices and exit.

> -Defaults to @option{false}.

> +Defaults to @option{false}. Alternatively you can use the @code{-sources}

> +option of ffmpeg to list the available input devices.

>  

>  @item list_formats

>  If set to @option{true}, print a list of supported formats and exit.

> diff --git a/doc/outdevs.texi b/doc/outdevs.texi

> index 34c508a970..2518f9b559 100644

> --- a/doc/outdevs.texi

> +++ b/doc/outdevs.texi

> @@ -140,7 +140,8 @@ device with @command{-list_formats 1}. Audio sample rate is always 48 kHz.

>  

>  @item list_devices

>  If set to @option{true}, print a list of devices and exit.

> -Defaults to @option{false}.

> +Defaults to @option{false}. Alternatively you can use the @code{-sinks}

> +option of ffmpeg to list the available output devices.

>  

>  @item list_formats

>  If set to @option{true}, print a list of supported formats and exit.

> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp

> index 503417bb35..b88d6c6219 100644

> --- a/libavdevice/decklink_common.cpp

> +++ b/libavdevice/decklink_common.cpp

> @@ -77,15 +77,25 @@ static IDeckLinkIterator *decklink_create_iterator(AVFormatContext *avctx)

>      return iter;

>  }

>  

> -HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName)

> +int decklink_get_attr_string(IDeckLink *dl, BMDDeckLinkAttributeID cfg_id, const char **s)

>  {

> -    DECKLINK_STR tmpDisplayName;

> -    HRESULT hr = This->GetDisplayName(&tmpDisplayName);

> -    if (hr != S_OK)

> -        return hr;

> -    *displayName = DECKLINK_STRDUP(tmpDisplayName);

> -    DECKLINK_FREE(tmpDisplayName);

> -    return hr;

> +    DECKLINK_STR tmp;

> +    HRESULT hr;

> +    IDeckLinkAttributes *attr;

> +    *s = NULL;

> +    if (dl->QueryInterface(IID_IDeckLinkAttributes, (void **)&attr) != S_OK)

> +        return AVERROR_EXTERNAL;

> +    hr = attr->GetString(cfg_id, &tmp);

> +    attr->Release();

> +    if (hr == S_OK) {

> +        *s = DECKLINK_STRDUP(tmp);

> +        DECKLINK_FREE(tmp);

> +        if (!*s)

> +            return AVERROR(ENOMEM);

> +    } else if (hr == E_FAIL) {

> +        return AVERROR_EXTERNAL;

> +    }

> +    return 0;

>  }

>  

>  static int decklink_select_input(AVFormatContext *avctx, BMDDeckLinkConfigurationID cfg_id)

> @@ -276,11 +286,17 @@ int ff_decklink_list_devices(AVFormatContext *avctx,

>      while (ret == 0 && iter->Next(&dl) == S_OK) {

>          IDeckLinkOutput *output_config;

>          IDeckLinkInput *input_config;

> -        const char *displayName;

> +        const char *display_name = NULL;

> +        const char *unique_name = NULL;

>          AVDeviceInfo *new_device = NULL;

>          int add = 0;

>  

> -        ff_decklink_get_display_name(dl, &displayName);

> +        ret = decklink_get_attr_string(dl, BMDDeckLinkDisplayName, &display_name);

> +        if (ret < 0)

> +            goto next;

> +        ret = decklink_get_attr_string(dl, BMDDeckLinkDeviceHandle, &unique_name);

> +        if (ret < 0)

> +            goto next;

>  

>          if (show_outputs) {

>              if (dl->QueryInterface(IID_IDeckLinkOutput, (void **)&output_config) == S_OK) {

> @@ -303,8 +319,8 @@ int ff_decklink_list_devices(AVFormatContext *avctx,

>                  goto next;

>              }

>  

> -            new_device->device_name = av_strdup(displayName);

> -            new_device->device_description = av_strdup(displayName);

> +            new_device->device_name = av_strdup(unique_name ? unique_name : display_name);

> +            new_device->device_description = av_strdup(display_name);

>  

>              if (!new_device->device_name ||

>                  !new_device->device_description ||

> @@ -318,7 +334,8 @@ int ff_decklink_list_devices(AVFormatContext *avctx,

>          }

>  

>      next:

> -        av_freep(&displayName);

> +        av_freep(&display_name);

> +        av_freep(&unique_name);

>          dl->Release();

>      }

>      iter->Release();

> @@ -343,7 +360,7 @@ void ff_decklink_list_devices_legacy(AVFormatContext *avctx,

>          av_log(avctx, AV_LOG_INFO, "Blackmagic DeckLink %s devices:\n",

>                 show_inputs ? "input" : "output");

>          for (int i = 0; i < device_list->nb_devices; i++) {

> -            av_log(avctx, AV_LOG_INFO, "\t'%s'\n", device_list->devices[i]->device_name);

> +            av_log(avctx, AV_LOG_INFO, "\t'%s'\n", device_list->devices[i]->device_description);

>          }

>      }

>      avdevice_free_list_devices(&device_list);

> @@ -427,14 +444,18 @@ int ff_decklink_init_device(AVFormatContext *avctx, const char* name)

>          return AVERROR_EXTERNAL;

>  

>      while (iter->Next(&dl) == S_OK) {

> -        const char *displayName;

> -        ff_decklink_get_display_name(dl, &displayName);

> -        if (!strcmp(name, displayName)) {

> -            av_free((void *)displayName);

> +        const char *display_name = NULL;

> +        const char *unique_name = NULL;

> +        decklink_get_attr_string(dl, BMDDeckLinkDisplayName, &display_name);

> +        decklink_get_attr_string(dl, BMDDeckLinkDeviceHandle, &unique_name);

> +        if (display_name && !strcmp(name, display_name) || unique_name && !strcmp(name, unique_name)) {

> +            av_free((void *)unique_name);

> +            av_free((void *)display_name);

>              ctx->dl = dl;

>              break;

>          }

> -        av_free((void *)displayName);

> +        av_free((void *)display_name);

> +        av_free((void *)unique_name);

>          dl->Release();

>      }

>      iter->Release();

> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h

> index 128144f50d..d2fc3f79d5 100644

> --- a/libavdevice/decklink_common.h

> +++ b/libavdevice/decklink_common.h

> @@ -191,7 +191,6 @@ static const BMDTimecodeFormat decklink_timecode_format_map[] = {

>      bmdTimecodeSerial,

>  };

>  

> -HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName);

>  int ff_decklink_set_configs(AVFormatContext *avctx, decklink_direction_t direction);

>  int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int tb_num, int tb_den, enum AVFieldOrder field_order, decklink_direction_t direction = DIRECTION_OUT, int num = 0);

>  int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t direction, int num);
Marton Balint Sept. 21, 2018, 11:40 a.m.
On Fri, 21 Sep 2018, Jeyapal, Karthick wrote:

>
> On 9/21/18 3:35 AM, Marton Balint wrote:
>> Also bump the API version requirement to 10.9.5, because on olders versions
>> there were some reports of crashes using the undocumented, yet available
>> BMDDeckLinkDeviceHandle.

> If we are using an undocumented API feature, then there is a higher risk 
> of this feature not working in some future versions of DeckLink drivers 
> as well. I would suggest using this undocumented API call under an 'if' 
> condition, which is controlled by a command line option for selecting 
> based on unique ID. In that way setups which prefers stability can 
> choose to not use this feature.

It got documented in 10.10, so this is no longer an issue. I tested it 
with 10.9.10 as there were (non-related) regressions in 10.10, and it 
worked fine there, so I figured it should be OK to require only 10.9. If 
you still think using a non-documented feature is shady, I can bump the 
requirement to 10.10.

Regards,
Marton
kjeyapal@akamai.com Sept. 22, 2018, 10:27 a.m.
On 9/21/18 5:10 PM, Marton Balint wrote:
>

>

> On Fri, 21 Sep 2018, Jeyapal, Karthick wrote:

>

>>

>> On 9/21/18 3:35 AM, Marton Balint wrote:

>>> Also bump the API version requirement to 10.9.5, because on olders versions

>>> there were some reports of crashes using the undocumented, yet available

>>> BMDDeckLinkDeviceHandle. 

>

>> If we are using an undocumented API feature, then there is a higher risk of this feature not working in some future versions of DeckLink drivers as well. I would suggest using this undocumented API call under an 'if' condition, which is controlled by a command line option for selecting based on unique ID. In that way setups which prefers stability can choose to not use this feature. 

>

> It got documented in 10.10, so this is no longer an issue. I tested it with 10.9.10 as there were (non-related) regressions in 10.10, and it worked fine there, so I figured it should be OK to require only 10.9. If you still think using a non-documented feature is shady, I can bump the requirement to 10.10.

Thanks for the clarification. In that case the current patch is fine, since you have testing it for 10.9.
>

> Regards,

> Marton

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Marton Balint Sept. 23, 2018, 5:47 p.m.
On Sat, 22 Sep 2018, Jeyapal, Karthick wrote:

>
> On 9/21/18 5:10 PM, Marton Balint wrote:
>>
>>
>> On Fri, 21 Sep 2018, Jeyapal, Karthick wrote:
>>
>>>
>>> On 9/21/18 3:35 AM, Marton Balint wrote:
>>>> Also bump the API version requirement to 10.9.5, because on olders versions
>>>> there were some reports of crashes using the undocumented, yet available
>>>> BMDDeckLinkDeviceHandle.
>>
>>> If we are using an undocumented API feature, then there is a higher 
>>> risk of this feature not working in some future versions of DeckLink 
>>> drivers as well. I would suggest using this undocumented API call 
>>> under an 'if' condition, which is controlled by a command line option 
>>> for selecting based on unique ID. In that way setups which prefers 
>>> stability can choose to not use this feature.
>>
>> It got documented in 10.10, so this is no longer an issue. I tested it 
>> with 10.9.10 as there were (non-related) regressions in 10.10, and it 
>> worked fine there, so I figured it should be OK to require only 10.9. 
>> If you still think using a non-documented feature is shady, I can bump 
>> the requirement to 10.10.

> Thanks for the clarification. In that case the current patch is fine, 
> since you have testing it for 10.9.

Thanks, applied.

Regards,
Marton

Patch hide | download patch | download mbox

diff --git a/configure b/configure
index 25120337de..64a519a8b3 100755
--- a/configure
+++ b/configure
@@ -6043,7 +6043,7 @@  done
 enabled cuda_sdk          && require cuda_sdk cuda.h cuCtxCreate -lcuda
 enabled chromaprint       && require chromaprint chromaprint.h chromaprint_get_version -lchromaprint
 enabled decklink          && { require_headers DeckLinkAPI.h &&
-                               { test_cpp_condition DeckLinkAPIVersion.h "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a060100" || die "ERROR: Decklink API version must be >= 10.6.1."; } }
+                               { test_cpp_condition DeckLinkAPIVersion.h "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a090500" || die "ERROR: Decklink API version must be >= 10.9.5."; } }
 enabled libndi_newtek     && require_headers Processing.NDI.Lib.h
 enabled frei0r            && require_headers frei0r.h
 enabled gmp               && require gmp gmp.h mpz_export -lgmp
diff --git a/doc/indevs.texi b/doc/indevs.texi
index 5d4c02c597..ed2784be9f 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -267,7 +267,8 @@  audio track.
 
 @item list_devices
 If set to @option{true}, print a list of devices and exit.
-Defaults to @option{false}.
+Defaults to @option{false}. Alternatively you can use the @code{-sources}
+option of ffmpeg to list the available input devices.
 
 @item list_formats
 If set to @option{true}, print a list of supported formats and exit.
diff --git a/doc/outdevs.texi b/doc/outdevs.texi
index 34c508a970..2518f9b559 100644
--- a/doc/outdevs.texi
+++ b/doc/outdevs.texi
@@ -140,7 +140,8 @@  device with @command{-list_formats 1}. Audio sample rate is always 48 kHz.
 
 @item list_devices
 If set to @option{true}, print a list of devices and exit.
-Defaults to @option{false}.
+Defaults to @option{false}. Alternatively you can use the @code{-sinks}
+option of ffmpeg to list the available output devices.
 
 @item list_formats
 If set to @option{true}, print a list of supported formats and exit.
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index 503417bb35..b88d6c6219 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -77,15 +77,25 @@  static IDeckLinkIterator *decklink_create_iterator(AVFormatContext *avctx)
     return iter;
 }
 
-HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName)
+int decklink_get_attr_string(IDeckLink *dl, BMDDeckLinkAttributeID cfg_id, const char **s)
 {
-    DECKLINK_STR tmpDisplayName;
-    HRESULT hr = This->GetDisplayName(&tmpDisplayName);
-    if (hr != S_OK)
-        return hr;
-    *displayName = DECKLINK_STRDUP(tmpDisplayName);
-    DECKLINK_FREE(tmpDisplayName);
-    return hr;
+    DECKLINK_STR tmp;
+    HRESULT hr;
+    IDeckLinkAttributes *attr;
+    *s = NULL;
+    if (dl->QueryInterface(IID_IDeckLinkAttributes, (void **)&attr) != S_OK)
+        return AVERROR_EXTERNAL;
+    hr = attr->GetString(cfg_id, &tmp);
+    attr->Release();
+    if (hr == S_OK) {
+        *s = DECKLINK_STRDUP(tmp);
+        DECKLINK_FREE(tmp);
+        if (!*s)
+            return AVERROR(ENOMEM);
+    } else if (hr == E_FAIL) {
+        return AVERROR_EXTERNAL;
+    }
+    return 0;
 }
 
 static int decklink_select_input(AVFormatContext *avctx, BMDDeckLinkConfigurationID cfg_id)
@@ -276,11 +286,17 @@  int ff_decklink_list_devices(AVFormatContext *avctx,
     while (ret == 0 && iter->Next(&dl) == S_OK) {
         IDeckLinkOutput *output_config;
         IDeckLinkInput *input_config;
-        const char *displayName;
+        const char *display_name = NULL;
+        const char *unique_name = NULL;
         AVDeviceInfo *new_device = NULL;
         int add = 0;
 
-        ff_decklink_get_display_name(dl, &displayName);
+        ret = decklink_get_attr_string(dl, BMDDeckLinkDisplayName, &display_name);
+        if (ret < 0)
+            goto next;
+        ret = decklink_get_attr_string(dl, BMDDeckLinkDeviceHandle, &unique_name);
+        if (ret < 0)
+            goto next;
 
         if (show_outputs) {
             if (dl->QueryInterface(IID_IDeckLinkOutput, (void **)&output_config) == S_OK) {
@@ -303,8 +319,8 @@  int ff_decklink_list_devices(AVFormatContext *avctx,
                 goto next;
             }
 
-            new_device->device_name = av_strdup(displayName);
-            new_device->device_description = av_strdup(displayName);
+            new_device->device_name = av_strdup(unique_name ? unique_name : display_name);
+            new_device->device_description = av_strdup(display_name);
 
             if (!new_device->device_name ||
                 !new_device->device_description ||
@@ -318,7 +334,8 @@  int ff_decklink_list_devices(AVFormatContext *avctx,
         }
 
     next:
-        av_freep(&displayName);
+        av_freep(&display_name);
+        av_freep(&unique_name);
         dl->Release();
     }
     iter->Release();
@@ -343,7 +360,7 @@  void ff_decklink_list_devices_legacy(AVFormatContext *avctx,
         av_log(avctx, AV_LOG_INFO, "Blackmagic DeckLink %s devices:\n",
                show_inputs ? "input" : "output");
         for (int i = 0; i < device_list->nb_devices; i++) {
-            av_log(avctx, AV_LOG_INFO, "\t'%s'\n", device_list->devices[i]->device_name);
+            av_log(avctx, AV_LOG_INFO, "\t'%s'\n", device_list->devices[i]->device_description);
         }
     }
     avdevice_free_list_devices(&device_list);
@@ -427,14 +444,18 @@  int ff_decklink_init_device(AVFormatContext *avctx, const char* name)
         return AVERROR_EXTERNAL;
 
     while (iter->Next(&dl) == S_OK) {
-        const char *displayName;
-        ff_decklink_get_display_name(dl, &displayName);
-        if (!strcmp(name, displayName)) {
-            av_free((void *)displayName);
+        const char *display_name = NULL;
+        const char *unique_name = NULL;
+        decklink_get_attr_string(dl, BMDDeckLinkDisplayName, &display_name);
+        decklink_get_attr_string(dl, BMDDeckLinkDeviceHandle, &unique_name);
+        if (display_name && !strcmp(name, display_name) || unique_name && !strcmp(name, unique_name)) {
+            av_free((void *)unique_name);
+            av_free((void *)display_name);
             ctx->dl = dl;
             break;
         }
-        av_free((void *)displayName);
+        av_free((void *)display_name);
+        av_free((void *)unique_name);
         dl->Release();
     }
     iter->Release();
diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 128144f50d..d2fc3f79d5 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -191,7 +191,6 @@  static const BMDTimecodeFormat decklink_timecode_format_map[] = {
     bmdTimecodeSerial,
 };
 
-HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName);
 int ff_decklink_set_configs(AVFormatContext *avctx, decklink_direction_t direction);
 int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int tb_num, int tb_den, enum AVFieldOrder field_order, decklink_direction_t direction = DIRECTION_OUT, int num = 0);
 int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t direction, int num);