| Message ID | 20180920220531.1044-1-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
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);
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
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
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
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);
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(-)