[FFmpeg-devel,v2,2/2] avcodec/videotoolbox: fix hevc hwaccel build on older macOS

From: Aman Gupta <aman@tmm1.net>

---
 configure                 | 5 ++++-
 libavcodec/videotoolbox.c | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

On 9/27/2017 7:19 PM, Aman Gupta wrote:
> From: Aman Gupta <aman@tmm1.net>
> 
> ---
>  configure                 | 5 ++++-
>  libavcodec/videotoolbox.c | 4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index d353e9d824..aae23bf38f 100755
> --- a/configure
> +++ b/configure
> @@ -2073,6 +2073,7 @@ TOOLCHAIN_FEATURES="
>  
>  TYPES_LIST="
>      CONDITION_VARIABLE_Ptr
> +    kCMVideoCodecType_HEVC

Remove this line and read below.

>      socklen_t
>      struct_addrinfo
>      struct_group_source_req
> @@ -5809,8 +5810,10 @@ enabled avfoundation && {
>      check_lib avfoundation CoreGraphics/CoreGraphics.h               CGGetActiveDisplayList "-framework CoreGraphics" ||
>      check_lib avfoundation ApplicationServices/ApplicationServices.h CGGetActiveDisplayList "-framework ApplicationServices"; }
>  
> -enabled videotoolbox &&
> +enabled videotoolbox && {
>      check_lib coreservices CoreServices/CoreServices.h UTGetOSTypeFromString "-framework CoreServices"
> +    check_func_headers CoreMedia/CMFormatDescription.h kCMVideoCodecType_HEVC "-framework CoreMedia"

Look how DXVA_PicParams_HEVC is checked and try to do the same instead.

> +}
>  
>  check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
>  
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index c96200cbdb..e62452c078 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -40,6 +40,10 @@
>  #  define kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder CFSTR("RequireHardwareAcceleratedVideoDecoder")
>  #endif
>  
> +#if !HAVE_KCMVIDEOCODECTYPE_HEVC
> +enum { kCMVideoCodecType_HEVC = 'hvc1' };
> +#endif

The correct thing to do is adding kCMVideoCodecType_HEVC to
hevc_videotoolbox_hwaccel_deps in configure, and not forcing it on SDKs
that don't support it since, i assume, no computer with MacOS 10.8 will
be able to play hevc videos anyway.

> +
>  #define VIDEOTOOLBOX_ESDS_EXTRADATA_PADDING  12
>  
>  static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
>

On Wed, Sep 27, 2017 at 3:52 PM, James Almer <jamrial@gmail.com> wrote:

> On 9/27/2017 7:19 PM, Aman Gupta wrote:
> > From: Aman Gupta <aman@tmm1.net>
> >
> > ---
> >  configure                 | 5 ++++-
> >  libavcodec/videotoolbox.c | 4 ++++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure b/configure
> > index d353e9d824..aae23bf38f 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2073,6 +2073,7 @@ TOOLCHAIN_FEATURES="
> >
> >  TYPES_LIST="
> >      CONDITION_VARIABLE_Ptr
> > +    kCMVideoCodecType_HEVC
>
> Remove this line and read below.
>
> >      socklen_t
> >      struct_addrinfo
> >      struct_group_source_req
> > @@ -5809,8 +5810,10 @@ enabled avfoundation && {
> >      check_lib avfoundation CoreGraphics/CoreGraphics.h
>  CGGetActiveDisplayList "-framework CoreGraphics" ||
> >      check_lib avfoundation ApplicationServices/ApplicationServices.h
> CGGetActiveDisplayList "-framework ApplicationServices"; }
> >
> > -enabled videotoolbox &&
> > +enabled videotoolbox && {
> >      check_lib coreservices CoreServices/CoreServices.h
> UTGetOSTypeFromString "-framework CoreServices"
> > +    check_func_headers CoreMedia/CMFormatDescription.h
> kCMVideoCodecType_HEVC "-framework CoreMedia"
>
> Look how DXVA_PicParams_HEVC is checked and try to do the same instead.
>

I don't think check_type will work because kCMVideoCodecType_HEVC is
defined as an enum.


>
> > +}
> >
> >  check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
> >
> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> > index c96200cbdb..e62452c078 100644
> > --- a/libavcodec/videotoolbox.c
> > +++ b/libavcodec/videotoolbox.c
> > @@ -40,6 +40,10 @@
> >  #  define kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder
> CFSTR("RequireHardwareAcceleratedVideoDecoder")
> >  #endif
> >
> > +#if !HAVE_KCMVIDEOCODECTYPE_HEVC
> > +enum { kCMVideoCodecType_HEVC = 'hvc1' };
> > +#endif
>
> The correct thing to do is adding kCMVideoCodecType_HEVC to
> hevc_videotoolbox_hwaccel_deps in configure, and not forcing it on SDKs
> that don't support it since, i assume, no computer with MacOS 10.8 will
> be able to play hevc videos anyway.
>

kCMVideoCodecType_HEVC is defined on 10.11 and 10.12, even though those
machines can't play hevc either. I don't think it should matter what mac
version you built ffmpeg on, only whether or not the machine where the
binary is being executed supports HEVC.

videotoolbox.c and videotoolboxenc.c both use runtime stubs for various
constants
like kVTVideoEncoderSpecification_RequireHardwareAcceleratedVideoEncoder
which are not defined consistently across different versions of macOS, so I
followed a similar pattern here.

That said, I don't have a strong preference and can make hevc_videotoolbox
depend on the symbol instead. My main goal is to allow binaries compiled on
10.12 to use HEVC on 10.13, and that will work with either configuration.


>
> > +
> >  #define VIDEOTOOLBOX_ESDS_EXTRADATA_PADDING  12
> >
> >  static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

On Wed, 27 Sep 2017 19:52:13 -0300
James Almer <jamrial@gmail.com> wrote:

> > +#if !HAVE_KCMVIDEOCODECTYPE_HEVC
> > +enum { kCMVideoCodecType_HEVC = 'hvc1' };
> > +#endif  
> 
> The correct thing to do is adding kCMVideoCodecType_HEVC to
> hevc_videotoolbox_hwaccel_deps in configure, and not forcing it on SDKs
> that don't support it since, i assume, no computer with MacOS 10.8 will
> be able to play hevc videos anyway.

SDK version != OS you build on != target machine

So this has some justification.

On 9/27/2017 8:11 PM, wm4 wrote:
> On Wed, 27 Sep 2017 19:52:13 -0300
> James Almer <jamrial@gmail.com> wrote:
> 
>>> +#if !HAVE_KCMVIDEOCODECTYPE_HEVC
>>> +enum { kCMVideoCodecType_HEVC = 'hvc1' };
>>> +#endif  
>>
>> The correct thing to do is adding kCMVideoCodecType_HEVC to
>> hevc_videotoolbox_hwaccel_deps in configure, and not forcing it on SDKs
>> that don't support it since, i assume, no computer with MacOS 10.8 will
>> be able to play hevc videos anyway.
> 
> SDK version != OS you build on != target machine
> 
> So this has some justification.

Neither is the case for dxva2 hevc and vp9, yet it's done like i
mentioned above.
Of course, those two require an entire header and not a single enum
value, so it probably explains the decision to make them dependencies.

I'm not going to block the patch for this, so implement it however you
prefer.