Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1760 closed defect (fixed)

Don't use VDA on Snow Leopard/ppc

Reported by: jeremyhu Owned by:
Priority: normal Component: undetermined
Version: git-master Keywords: vda ppc
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

In determining whether or not to use VDA, configure just checks for the header's existence:

# check for VDA header
if ! disabled vda; then
    if check_header VideoDecodeAcceleration/VDADecoder.h; then
        enable vda
        add_extralibs -framework CoreFoundation -framework VideoDecodeAcceleration -framework QuartzCore
    fi   
fi

but VideoDecodeAcceleration? is intel-only and will cause ppc slices to fail to build.

Change History (8)

comment:1 Changed 5 years ago by jeremyhu

  • Component changed from undetermined to avcodec
  • Keywords vda added

It doesn't seem right to disable VDA across all slices just because it's not available on ppc, so I would suggest updating with a patch like:

diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 8806c6a..b15b784 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -58,7 +58,9 @@ void avcodec_register_all(void)
     REGISTER_HWACCEL (H263_VAAPI, h263_vaapi);
     REGISTER_HWACCEL (H264_DXVA2, h264_dxva2);
     REGISTER_HWACCEL (H264_VAAPI, h264_vaapi);
+#ifndef __ppc__
     REGISTER_HWACCEL (H264_VDA, h264_vda);
+#endif
     REGISTER_HWACCEL (MPEG1_VDPAU, mpeg1_vdpau);
     REGISTER_HWACCEL (MPEG2_DXVA2, mpeg2_dxva2);
     REGISTER_HWACCEL (MPEG2_VAAPI, mpeg2_vaapi);
@@ -135,7 +137,9 @@ void avcodec_register_all(void)
     REGISTER_ENCDEC  (H263P, h263p);
     REGISTER_DECODER (H264, h264);
     REGISTER_DECODER (H264_CRYSTALHD, h264_crystalhd);
+#ifndef __ppc__
     REGISTER_DECODER (H264_VDA, h264_vda);
+#endif
     REGISTER_DECODER (H264_VDPAU, h264_vdpau);
     REGISTER_ENCDEC  (HUFFYUV, huffyuv);
     REGISTER_DECODER (IDCIN, idcin);
diff --git a/libavcodec/vda_h264.c b/libavcodec/vda_h264.c
index 4b8c754..fe620bc 100644
--- a/libavcodec/vda_h264.c
+++ b/libavcodec/vda_h264.c
@@ -20,6 +20,9 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* https://ffmpeg.org/trac/ffmpeg/ticket/1760 */
+#ifndef __ppc__
+
 #include <CoreFoundation/CFDictionary.h>
 #include <CoreFoundation/CFNumber.h>
 #include <CoreFoundation/CFData.h>
@@ -380,3 +383,5 @@ AVHWAccel ff_h264_vda_hwaccel = {
     .decode_slice   = decode_slice,
     .end_frame      = end_frame,
 };
+
+#endif /* !ppc */
diff --git a/libavcodec/vda_h264_dec.c b/libavcodec/vda_h264_dec.c
index 36453c5..09115a7 100644
--- a/libavcodec/vda_h264_dec.c
+++ b/libavcodec/vda_h264_dec.c
@@ -24,6 +24,9 @@
  * @author Xidorn Quan <quanxunzhen@gmail.com>
  */
 
+/* https://ffmpeg.org/trac/ffmpeg/ticket/1760 */
+#ifndef __ppc__
+
 #include <string.h>
 #include <CoreFoundation/CoreFoundation.h>
 
@@ -256,3 +259,5 @@ AVCodec ff_h264_vda_decoder = {
     .flush          = vdadec_flush,
     .long_name      = NULL_IF_CONFIG_SMALL("H.264 (VDA acceleration)"),
 };
+
+#endif /* !ppc */

This caused the darwin/ppc fate builds to start failing when I updated them from Leopard to Snow Leopard:
http://fate.ffmpeg.org/history.cgi?slot=ppc-darwin-gcc-4.2.1

comment:2 Changed 5 years ago by cehoyos

  • Component changed from avcodec to undetermined
  • Status changed from new to open
  • Version changed from unspecified to git-master

Imo, this should definitely be fixed in configure, not in the library.

comment:3 Changed 5 years ago by jeremyhu

That would only be possible if you can do per-arch choices into config.h, but it doesn't look like your build system supports this:

#ifdef ppc
#define CONFIG_H264_VDA_DECODER 0
#define CONFIG_H264_VDA_HWACCEL 0
#else
#define CONFIG_H264_VDA_DECODER 1
#define CONFIG_H264_VDA_HWACCEL 1
#endif

comment:4 Changed 5 years ago by jeremyhu

Ie, you need to compile *different* C code for the different archs at the same time... the only way to do this is by changing the library to do something different for ppc than for intel...

comment:5 Changed 5 years ago by jeremyhu

The ffmpeg build system is already broken in other areas for universal builds, but I'd rather not add to that problem...

comment:6 follow-up: Changed 5 years ago by reimar

It is not broken, it is behaving exactly as intended. If you want two architectures, build twice. You can still merge them afterwards to get a universal binary.

comment:7 Changed 5 years ago by michael

  • Keywords ppc added
  • Resolution set to fixed
  • Status changed from open to closed

Fixed by Sebastien

comment:8 in reply to: ↑ 6 Changed 5 years ago by jeremyhu

Replying to reimar:

It is not broken, it is behaving exactly as intended. If you want two architectures, build twice. You can still merge them afterwards to get a universal binary.

broken by design is still broken.

Note: See TracTickets for help on using tickets.