Message ID | 20170927221920.46302-2-ffmpeg@tmm1.net |
---|---|
State | New |
Headers | show |
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.
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 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" +} 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 + #define VIDEOTOOLBOX_ESDS_EXTRADATA_PADDING 12 static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
From: Aman Gupta <aman@tmm1.net> --- configure | 5 ++++- libavcodec/videotoolbox.c | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-)