diff mbox series

[FFmpeg-devel,v2] avutil/hwcontext_videotoolbox: Check CVBufferCopyAttachments during configure

Message ID 20240721224755.19964-1-gnattuoc@me.com
State New
Headers show
Series [FFmpeg-devel,v2] avutil/hwcontext_videotoolbox: Check CVBufferCopyAttachments during configure | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Gnattu OC July 21, 2024, 10:47 p.m. UTC
The __builtin_available function does not do compile time check
for the availablity of the CVBufferCopyAttachments function
which will fail the build. Check the availability during configure.

Signed-off-by: Gnattu OC <gnattuoc@me.com>
---
 configure                          |  2 ++
 libavutil/hwcontext_videotoolbox.c | 12 +++++-------
 2 files changed, 7 insertions(+), 7 deletions(-)

Comments

Marvin Scholz July 22, 2024, 8:55 a.m. UTC | #1
On 22 Jul 2024, at 0:47, gnattu via ffmpeg-devel wrote:

> The __builtin_available function does not do compile time check
> for the availablity of the CVBufferCopyAttachments function
> which will fail the build. Check the availability during configure.
>
> Signed-off-by: Gnattu OC <gnattuoc@me.com>
> ---
>  configure                          |  2 ++
>  libavutil/hwcontext_videotoolbox.c | 12 +++++-------
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/configure b/configure
> index f6f5c29fea..54171dd4e5 100755
> --- a/configure
> +++ b/configure
> @@ -2366,6 +2366,7 @@ SYSTEM_FUNCS="
>      clock_gettime
>      closesocket
>      CommandLineToArgvW
> +    CVBufferCopyAttachments
>      fcntl
>      getaddrinfo
>      getauxval
> @@ -6684,6 +6685,7 @@ enabled videotoolbox && {
>      check_func_headers CoreVideo/CVImageBuffer.h kCVImageBufferColorPrimaries_ITU_R_2020 "-framework CoreVideo"
>      check_func_headers CoreVideo/CVImageBuffer.h kCVImageBufferTransferFunction_ITU_R_2020 "-framework CoreVideo"
>      check_func_headers CoreVideo/CVImageBuffer.h kCVImageBufferTransferFunction_SMPTE_ST_428_1 "-framework CoreVideo"
> +    check_func_headers CoreVideo/CVBuffer.h CVBufferCopyAttachments "-framework CoreVideo"
>  }
>

Thanks for the fix and sorry for breaking this.

I am not too keen on adding a configure check just for that, as if we do that for all partially available function
it will make configure even slower than it already is.

I have a fix for this ready testing for the right SDK conditionals (I only did not catch it before because
the way the SDK annotations work nowadays do not seem to properly honor when I set my own __MAC_OS_X_VERSION_MAX_ALLOWED
define…) so I will need to actually test this on an old enough SDK just to be sure, this time…

Then again, if others prefer doing it with configure tests, I am ok with that as well.

>  enabled metal && test_cmd $metalcc -v || disable metal
> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
> index ab7556936d..c55d478004 100644
> --- a/libavutil/hwcontext_videotoolbox.c
> +++ b/libavutil/hwcontext_videotoolbox.c
> @@ -592,15 +592,13 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>      (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
>      if (__builtin_available(macOS 10.8, iOS 10, *)) {
>          CFDictionaryRef attachments = NULL;
> +#if HAVE_CVBUFFERCOPYATTACHMENTS
>          if (__builtin_available(macOS 12.0, iOS 15.0, *))
>              attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
> -#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MIN_REQUIRED <= 120000) || \
> -    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MIN_REQUIRED <= 150000)
> -        else {
> -            CFDictionaryRef tmp = CVBufferGetAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
> -            if (tmp)
> -                attachments = CFDictionaryCreateCopy(NULL, tmp);
> -        }
> +#else
> +        CFDictionaryRef tmp = CVBufferGetAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
> +        if (tmp)
> +            attachments = CFDictionaryCreateCopy(NULL, tmp);
>  #endif
>          if (attachments) {
>              colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
> -- 
> 2.39.3 (Apple Git-146)
>
> _______________________________________________
> 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/configure b/configure
index f6f5c29fea..54171dd4e5 100755
--- a/configure
+++ b/configure
@@ -2366,6 +2366,7 @@  SYSTEM_FUNCS="
     clock_gettime
     closesocket
     CommandLineToArgvW
+    CVBufferCopyAttachments
     fcntl
     getaddrinfo
     getauxval
@@ -6684,6 +6685,7 @@  enabled videotoolbox && {
     check_func_headers CoreVideo/CVImageBuffer.h kCVImageBufferColorPrimaries_ITU_R_2020 "-framework CoreVideo"
     check_func_headers CoreVideo/CVImageBuffer.h kCVImageBufferTransferFunction_ITU_R_2020 "-framework CoreVideo"
     check_func_headers CoreVideo/CVImageBuffer.h kCVImageBufferTransferFunction_SMPTE_ST_428_1 "-framework CoreVideo"
+    check_func_headers CoreVideo/CVBuffer.h CVBufferCopyAttachments "-framework CoreVideo"
 }
 
 enabled metal && test_cmd $metalcc -v || disable metal
diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
index ab7556936d..c55d478004 100644
--- a/libavutil/hwcontext_videotoolbox.c
+++ b/libavutil/hwcontext_videotoolbox.c
@@ -592,15 +592,13 @@  static int vt_pixbuf_set_colorspace(void *log_ctx,
     (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
     if (__builtin_available(macOS 10.8, iOS 10, *)) {
         CFDictionaryRef attachments = NULL;
+#if HAVE_CVBUFFERCOPYATTACHMENTS
         if (__builtin_available(macOS 12.0, iOS 15.0, *))
             attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
-#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MIN_REQUIRED <= 120000) || \
-    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MIN_REQUIRED <= 150000)
-        else {
-            CFDictionaryRef tmp = CVBufferGetAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
-            if (tmp)
-                attachments = CFDictionaryCreateCopy(NULL, tmp);
-        }
+#else
+        CFDictionaryRef tmp = CVBufferGetAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
+        if (tmp)
+            attachments = CFDictionaryCreateCopy(NULL, tmp);
 #endif
         if (attachments) {
             colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);