diff mbox series

[FFmpeg-devel,11/15] lavc/videotoolbox: call VTRegisterSupplementalVideoDecoderIfAvailable

Message ID 20211113210916.49167-11-rcombs@rcombs.me
State New
Headers show
Series [FFmpeg-devel,01/15] lavu/pixfmt: add high-bit-depth semi-planar 4:2:2/4:4:4 formats | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

rcombs Nov. 13, 2021, 9:09 p.m. UTC 
This is required for VP9 to work.
---
 libavcodec/videotoolbox.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Marvin Scholz Nov. 14, 2021, 12:38 a.m. UTC | #1 
On 13 Nov 2021, at 22:09, rcombs wrote:

> This is required for VP9 to work.
> ---
>  libavcodec/videotoolbox.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index 18cc589d2a..0666446dbd 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -32,6 +32,7 @@
>  #include "h264dec.h"
>  #include "hevcdec.h"
>  #include "mpegvideo.h"
> +#include <Availability.h>
>  #include <TargetConditionals.h>
>
>  #ifndef 
> kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
> @@ -864,6 +865,12 @@ static int videotoolbox_start(AVCodecContext 
> *avctx)
>          break;
>      }
>
> +#ifdef __MAC_10_11
> +    if (__builtin_available(macOS 10.11, *)) {
> +        
> VTRegisterSupplementalVideoDecoderIfAvailable(videotoolbox->cm_codec_type);
> +    }
> +#endif
> +

The VTRegisterSupplementalVideoDecoderIfAvailable is available since 
macOS 11 according to the
header annotations:

VT_EXPORT void VTRegisterSupplementalVideoDecoderIfAvailable( 
CMVideoCodecType codecType ) API_AVAILABLE(macosx(11.0)) 
API_UNAVAILABLE(ios, watchos, tvos);

I guess you meant to check for macOS 11 here but accidentally used 
10.11?

Additionally a more reliable/correct way for the SDK preprocessor check 
would be:

#if (!TARGET_OS_IPHONE && MAC_OS_X_VERSION_MAX_ALLOWED >= 110000)

>      decoder_spec = 
> videotoolbox_decoder_config_create(videotoolbox->cm_codec_type, 
> avctx);
>
>      if (!decoder_spec) {
> -- 
> 2.33.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".
rcombs Nov. 14, 2021, 2:40 a.m. UTC | #2 
> On Nov 13, 2021, at 18:38, Marvin Scholz <epirat07@gmail.com> wrote:
> 
> On 13 Nov 2021, at 22:09, rcombs wrote:
> 
>> This is required for VP9 to work.
>> ---
>> libavcodec/videotoolbox.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>> 
>> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
>> index 18cc589d2a..0666446dbd 100644
>> --- a/libavcodec/videotoolbox.c
>> +++ b/libavcodec/videotoolbox.c
>> @@ -32,6 +32,7 @@
>> #include "h264dec.h"
>> #include "hevcdec.h"
>> #include "mpegvideo.h"
>> +#include <Availability.h>
>> #include <TargetConditionals.h>
>> 
>> #ifndef kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
>> @@ -864,6 +865,12 @@ static int videotoolbox_start(AVCodecContext *avctx)
>>         break;
>>     }
>> 
>> +#ifdef __MAC_10_11
>> +    if (__builtin_available(macOS 10.11, *)) {
>> +        VTRegisterSupplementalVideoDecoderIfAvailable(videotoolbox->cm_codec_type);
>> +    }
>> +#endif
>> +
> 
> The VTRegisterSupplementalVideoDecoderIfAvailable is available since macOS 11 according to the
> header annotations:
> 
> VT_EXPORT void VTRegisterSupplementalVideoDecoderIfAvailable( CMVideoCodecType codecType ) API_AVAILABLE(macosx(11.0)) API_UNAVAILABLE(ios, watchos, tvos);
> 
> I guess you meant to check for macOS 11 here but accidentally used 10.11?

Ah, good catch! Fixed.

> 
> Additionally a more reliable/correct way for the SDK preprocessor check would be:
> 
> #if (!TARGET_OS_IPHONE && MAC_OS_X_VERSION_MAX_ALLOWED >= 110000)

How's this, using AvailabilityMacros.h?

#if defined(MAC_OS_VERSION_11_0) && !TARGET_OS_IPHONE && (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0)

(And same for the 10.9 check)

> 
>>     decoder_spec = videotoolbox_decoder_config_create(videotoolbox->cm_codec_type, avctx);
>> 
>>     if (!decoder_spec) {
>> -- 
>> 2.33.1
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <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 <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".
Marvin Scholz Nov. 14, 2021, 3:59 a.m. UTC | #3 
On 14 Nov 2021, at 3:40, Ridley Combs wrote:

>> On Nov 13, 2021, at 18:38, Marvin Scholz <epirat07@gmail.com> wrote:
>>
>> On 13 Nov 2021, at 22:09, rcombs wrote:
>>
>>> This is required for VP9 to work.
>>> ---
>>> libavcodec/videotoolbox.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
>>> index 18cc589d2a..0666446dbd 100644
>>> --- a/libavcodec/videotoolbox.c
>>> +++ b/libavcodec/videotoolbox.c
>>> @@ -32,6 +32,7 @@
>>> #include "h264dec.h"
>>> #include "hevcdec.h"
>>> #include "mpegvideo.h"
>>> +#include <Availability.h>
>>> #include <TargetConditionals.h>
>>>
>>> #ifndef 
>>> kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
>>> @@ -864,6 +865,12 @@ static int videotoolbox_start(AVCodecContext 
>>> *avctx)
>>>         break;
>>>     }
>>>
>>> +#ifdef __MAC_10_11
>>> +    if (__builtin_available(macOS 10.11, *)) {
>>> +        
>>> VTRegisterSupplementalVideoDecoderIfAvailable(videotoolbox->cm_codec_type);
>>> +    }
>>> +#endif
>>> +
>>
>> The VTRegisterSupplementalVideoDecoderIfAvailable is available since 
>> macOS 11 according to the
>> header annotations:
>>
>> VT_EXPORT void VTRegisterSupplementalVideoDecoderIfAvailable( 
>> CMVideoCodecType codecType ) API_AVAILABLE(macosx(11.0)) 
>> API_UNAVAILABLE(ios, watchos, tvos);
>>
>> I guess you meant to check for macOS 11 here but accidentally used 
>> 10.11?
>
> Ah, good catch! Fixed.
>
>>
>> Additionally a more reliable/correct way for the SDK preprocessor 
>> check would be:
>>
>> #if (!TARGET_OS_IPHONE && MAC_OS_X_VERSION_MAX_ALLOWED >= 110000)
>
> How's this, using AvailabilityMacros.h?
>
> #if defined(MAC_OS_VERSION_11_0) && !TARGET_OS_IPHONE && 
> (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0)
>
> (And same for the 10.9 check)

Sure, good idea.

>
>>
>>>     decoder_spec = 
>>> videotoolbox_decoder_config_create(videotoolbox->cm_codec_type, 
>>> avctx);
>>>
>>>     if (!decoder_spec) {
>>> -- 
>>> 2.33.1
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
>>> <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 
>> <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
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes Nov. 14, 2021, 8:41 a.m. UTC | #4 
On Sun, Nov 14, 2021 at 3:40 AM Ridley Combs <rcombs@rcombs.me> wrote:
>
>
>
> > On Nov 13, 2021, at 18:38, Marvin Scholz <epirat07@gmail.com> wrote:
> >
> > On 13 Nov 2021, at 22:09, rcombs wrote:
> >
> >> This is required for VP9 to work.
> >> ---
> >> libavcodec/videotoolbox.c | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> >> index 18cc589d2a..0666446dbd 100644
> >> --- a/libavcodec/videotoolbox.c
> >> +++ b/libavcodec/videotoolbox.c
> >> @@ -32,6 +32,7 @@
> >> #include "h264dec.h"
> >> #include "hevcdec.h"
> >> #include "mpegvideo.h"
> >> +#include <Availability.h>
> >> #include <TargetConditionals.h>
> >>
> >> #ifndef kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
> >> @@ -864,6 +865,12 @@ static int videotoolbox_start(AVCodecContext *avctx)
> >>         break;
> >>     }
> >>
> >> +#ifdef __MAC_10_11
> >> +    if (__builtin_available(macOS 10.11, *)) {
> >> +        VTRegisterSupplementalVideoDecoderIfAvailable(videotoolbox->cm_codec_type);
> >> +    }
> >> +#endif
> >> +
> >
> > The VTRegisterSupplementalVideoDecoderIfAvailable is available since macOS 11 according to the
> > header annotations:
> >
> > VT_EXPORT void VTRegisterSupplementalVideoDecoderIfAvailable( CMVideoCodecType codecType ) API_AVAILABLE(macosx(11.0)) API_UNAVAILABLE(ios, watchos, tvos);
> >
> > I guess you meant to check for macOS 11 here but accidentally used 10.11?
>
> Ah, good catch! Fixed.
>
> >
> > Additionally a more reliable/correct way for the SDK preprocessor check would be:
> >
> > #if (!TARGET_OS_IPHONE && MAC_OS_X_VERSION_MAX_ALLOWED >= 110000)
>
> How's this, using AvailabilityMacros.h?
>
> #if defined(MAC_OS_VERSION_11_0) && !TARGET_OS_IPHONE && (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0)
>

MAX_ALLOWED seems like the wrong variable to use for me. Shouldn't
this be MAC_OS_X_VERSION_MIN_REQUIRED (or, both), so that I can still
target 10.15 or whatever with -mmacosx-version-min=10.15 without
linking in unavailable functions?

- Hendrik
rcombs Nov. 14, 2021, 9:11 a.m. UTC | #5 
> On Nov 14, 2021, at 02:41, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> 
> On Sun, Nov 14, 2021 at 3:40 AM Ridley Combs <rcombs@rcombs.me <mailto:rcombs@rcombs.me>> wrote:
>> 
>> 
>> 
>>> On Nov 13, 2021, at 18:38, Marvin Scholz <epirat07@gmail.com> wrote:
>>> 
>>> On 13 Nov 2021, at 22:09, rcombs wrote:
>>> 
>>>> This is required for VP9 to work.
>>>> ---
>>>> libavcodec/videotoolbox.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>> 
>>>> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
>>>> index 18cc589d2a..0666446dbd 100644
>>>> --- a/libavcodec/videotoolbox.c
>>>> +++ b/libavcodec/videotoolbox.c
>>>> @@ -32,6 +32,7 @@
>>>> #include "h264dec.h"
>>>> #include "hevcdec.h"
>>>> #include "mpegvideo.h"
>>>> +#include <Availability.h>
>>>> #include <TargetConditionals.h>
>>>> 
>>>> #ifndef kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
>>>> @@ -864,6 +865,12 @@ static int videotoolbox_start(AVCodecContext *avctx)
>>>>        break;
>>>>    }
>>>> 
>>>> +#ifdef __MAC_10_11
>>>> +    if (__builtin_available(macOS 10.11, *)) {
>>>> +        VTRegisterSupplementalVideoDecoderIfAvailable(videotoolbox->cm_codec_type);
>>>> +    }
>>>> +#endif
>>>> +
>>> 
>>> The VTRegisterSupplementalVideoDecoderIfAvailable is available since macOS 11 according to the
>>> header annotations:
>>> 
>>> VT_EXPORT void VTRegisterSupplementalVideoDecoderIfAvailable( CMVideoCodecType codecType ) API_AVAILABLE(macosx(11.0)) API_UNAVAILABLE(ios, watchos, tvos);
>>> 
>>> I guess you meant to check for macOS 11 here but accidentally used 10.11?
>> 
>> Ah, good catch! Fixed.
>> 
>>> 
>>> Additionally a more reliable/correct way for the SDK preprocessor check would be:
>>> 
>>> #if (!TARGET_OS_IPHONE && MAC_OS_X_VERSION_MAX_ALLOWED >= 110000)
>> 
>> How's this, using AvailabilityMacros.h?
>> 
>> #if defined(MAC_OS_VERSION_11_0) && !TARGET_OS_IPHONE && (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0)
>> 
> 
> MAX_ALLOWED seems like the wrong variable to use for me. Shouldn't
> this be MAC_OS_X_VERSION_MIN_REQUIRED (or, both), so that I can still
> target 10.15 or whatever with -mmacosx-version-min=10.15 without
> linking in unavailable functions?

The calls are behind __builtin_available, so they'll be weak-linked and only execute on macOS versions that include the relevant functions. MAX_ALLOWED is meant to indicate the latest version you want to even weakly link functions from (it doesn't actually mean the maximum supported version).

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <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/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 18cc589d2a..0666446dbd 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -32,6 +32,7 @@ 
 #include "h264dec.h"
 #include "hevcdec.h"
 #include "mpegvideo.h"
+#include <Availability.h>
 #include <TargetConditionals.h>
 
 #ifndef kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
@@ -864,6 +865,12 @@  static int videotoolbox_start(AVCodecContext *avctx)
         break;
     }
 
+#ifdef __MAC_10_11
+    if (__builtin_available(macOS 10.11, *)) {
+        VTRegisterSupplementalVideoDecoderIfAvailable(videotoolbox->cm_codec_type);
+    }
+#endif
+
     decoder_spec = videotoolbox_decoder_config_create(videotoolbox->cm_codec_type, avctx);
 
     if (!decoder_spec) {