diff mbox series

[FFmpeg-devel] avutil/hwcontext_videotoolbox: Fix build with older SDKs

Message ID D2L39E9Q06Y8.33NN74KBGJMVU@gmail.com
State New
Headers show
Series [FFmpeg-devel] avutil/hwcontext_videotoolbox: Fix build with older SDKs | expand

Checks

Context Check Description
andriy/configure_x86 warning Failed to apply patch
yinshiyou/configure_loongarch64 warning Failed to apply patch

Commit Message

Marvin Scholz July 9, 2024, 2:46 p.m. UTC 
I've accidentally used API not available on the checked version.
Additionally check for the SDK to be new enough to even have the
CVImageBufferCreateColorSpaceFromAttachments API to not fail
the build.
---
 libavutil/hwcontext_videotoolbox.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


base-commit: 9fb8d13d56f20728141fd7070d8a325720727d57

Comments

Zhao Zhili July 16, 2024, 12:11 p.m. UTC | #1 
> On Jul 9, 2024, at 22:46, Marvin Scholz <epirat07@gmail.com> wrote:
> 
> I've accidentally used API not available on the checked version.
> Additionally check for the SDK to be new enough to even have the
> CVImageBufferCreateColorSpaceFromAttachments API to not fail
> the build.
> ---
> libavutil/hwcontext_videotoolbox.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
> index 953155ce32..ab7556936d 100644
> --- a/libavutil/hwcontext_videotoolbox.c
> +++ b/libavutil/hwcontext_videotoolbox.c
> @@ -588,13 +588,26 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>     } else
>         CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
> 
> +#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
> +    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
>     if (__builtin_available(macOS 10.8, iOS 10, *)) {
> -        CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
> +        CFDictionaryRef attachments = NULL;
> +        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);
> +        }
> +#endif
>         if (attachments) {
>             colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
>             CFRelease(attachments);
>         }
>     }
> +#endif
> 
>     if (colorspace) {
>         CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey,

Rebase and applied, thanks.

> 
> base-commit: 9fb8d13d56f20728141fd7070d8a325720727d57
> -- 
> 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".
Sean McGovern July 21, 2024, 10:04 p.m. UTC | #2 
Hi Marvin,


On Tue, Jul 9, 2024, 10:47 Marvin Scholz <epirat07@gmail.com> wrote:

> I've accidentally used API not available on the checked version.
> Additionally check for the SDK to be new enough to even have the
> CVImageBufferCreateColorSpaceFromAttachments API to not fail
> the build.
> ---
>  libavutil/hwcontext_videotoolbox.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/hwcontext_videotoolbox.c
> b/libavutil/hwcontext_videotoolbox.c
> index 953155ce32..ab7556936d 100644
> --- a/libavutil/hwcontext_videotoolbox.c
> +++ b/libavutil/hwcontext_videotoolbox.c
> @@ -588,13 +588,26 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>      } else
>          CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
>
> +#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
> +    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
>      if (__builtin_available(macOS 10.8, iOS 10, *)) {
> -        CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf,
> kCVAttachmentMode_ShouldPropagate);
> +        CFDictionaryRef attachments = NULL;
> +        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);
> +        }
> +#endif
>          if (attachments) {
>              colorspace =
> CVImageBufferCreateColorSpaceFromAttachments(attachments);
>              CFRelease(attachments);
>          }
>      }
> +#endif
>
>      if (colorspace) {
>          CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey,
>
> base-commit: 9fb8d13d56f20728141fd7070d8a325720727d57
> --
> 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".
>

Should this have fixed the Apple M1 FATE node [1]? It seems to be still
failing.

-- Sean McGovern

[1] https://fate.ffmpeg.org/history.cgi?slot=aarch64-osx-clang-1200.0.32.29

>
Gnattu OC July 21, 2024, 10:45 p.m. UTC | #3 
> On Jul 22, 2024, at 06:04, Sean McGovern <gseanmcg@gmail.com> wrote:
> 
> Hi Marvin,
> 
> 
> On Tue, Jul 9, 2024, 10:47 Marvin Scholz <epirat07@gmail.com> wrote:
> 
>> I've accidentally used API not available on the checked version.
>> Additionally check for the SDK to be new enough to even have the
>> CVImageBufferCreateColorSpaceFromAttachments API to not fail
>> the build.
>> ---
>> libavutil/hwcontext_videotoolbox.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavutil/hwcontext_videotoolbox.c
>> b/libavutil/hwcontext_videotoolbox.c
>> index 953155ce32..ab7556936d 100644
>> --- a/libavutil/hwcontext_videotoolbox.c
>> +++ b/libavutil/hwcontext_videotoolbox.c
>> @@ -588,13 +588,26 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>>     } else
>>         CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
>> 
>> +#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
>> +    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
>>     if (__builtin_available(macOS 10.8, iOS 10, *)) {
>> -        CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf,
>> kCVAttachmentMode_ShouldPropagate);
>> +        CFDictionaryRef attachments = NULL;
>> +        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);
>> +        }
>> +#endif
>>         if (attachments) {
>>             colorspace =
>> CVImageBufferCreateColorSpaceFromAttachments(attachments);
>>             CFRelease(attachments);
>>         }
>>     }
>> +#endif
>> 
>>     if (colorspace) {
>>         CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey,
>> 
>> base-commit: 9fb8d13d56f20728141fd7070d8a325720727d57
>> --
>> 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".
>> 
> 
> Should this have fixed the Apple M1 FATE node [1]? It seems to be still
> failing.
> 
> -- Sean McGovern
> 
> [1] https://fate.ffmpeg.org/history.cgi?slot=aarch64-osx-clang-1200.0.32.29

I sent another patch that does the availability check at configure time which should fix the fate CI:
https://ffmpeg.org//pipermail/ffmpeg-devel/2024-July/331496.html

> 
>> 
> _______________________________________________
> 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".
Gnattu OC July 21, 2024, 10:48 p.m. UTC | #4 
> On Jul 22, 2024, at 06:45, Gnattu OC via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
> 
> 
> 
>> On Jul 22, 2024, at 06:04, Sean McGovern <gseanmcg@gmail.com> wrote:
>> 
>> Hi Marvin,
>> 
>> 
>> On Tue, Jul 9, 2024, 10:47 Marvin Scholz <epirat07@gmail.com> wrote:
>> 
>>> I've accidentally used API not available on the checked version.
>>> Additionally check for the SDK to be new enough to even have the
>>> CVImageBufferCreateColorSpaceFromAttachments API to not fail
>>> the build.
>>> ---
>>> libavutil/hwcontext_videotoolbox.c | 15 ++++++++++++++-
>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/libavutil/hwcontext_videotoolbox.c
>>> b/libavutil/hwcontext_videotoolbox.c
>>> index 953155ce32..ab7556936d 100644
>>> --- a/libavutil/hwcontext_videotoolbox.c
>>> +++ b/libavutil/hwcontext_videotoolbox.c
>>> @@ -588,13 +588,26 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>>>    } else
>>>        CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
>>> 
>>> +#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
>>> +    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
>>>    if (__builtin_available(macOS 10.8, iOS 10, *)) {
>>> -        CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf,
>>> kCVAttachmentMode_ShouldPropagate);
>>> +        CFDictionaryRef attachments = NULL;
>>> +        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);
>>> +        }
>>> +#endif
>>>        if (attachments) {
>>>            colorspace =
>>> CVImageBufferCreateColorSpaceFromAttachments(attachments);
>>>            CFRelease(attachments);
>>>        }
>>>    }
>>> +#endif
>>> 
>>>    if (colorspace) {
>>>        CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey,
>>> 
>>> base-commit: 9fb8d13d56f20728141fd7070d8a325720727d57
>>> --
>>> 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".
>>> 
>> 
>> Should this have fixed the Apple M1 FATE node [1]? It seems to be still
>> failing.
>> 
>> -- Sean McGovern
>> 
>> [1] https://fate.ffmpeg.org/history.cgi?slot=aarch64-osx-clang-1200.0.32.29
> 
> I sent another patch that does the availability check at configure time which should fix the fate CI:
> https://ffmpeg.org//pipermail/ffmpeg-devel/2024-July/331496.html
> 
Sorry, should be this one: https://ffmpeg.org//pipermail/ffmpeg-devel/2024-July/331499.html
>> 
>>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
index 953155ce32..ab7556936d 100644
--- a/libavutil/hwcontext_videotoolbox.c
+++ b/libavutil/hwcontext_videotoolbox.c
@@ -588,13 +588,26 @@  static int vt_pixbuf_set_colorspace(void *log_ctx,
     } else
         CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
 
+#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
+    (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
     if (__builtin_available(macOS 10.8, iOS 10, *)) {
-        CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
+        CFDictionaryRef attachments = NULL;
+        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);
+        }
+#endif
         if (attachments) {
             colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
             CFRelease(attachments);
         }
     }
+#endif
 
     if (colorspace) {
         CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey,