Opened 10 years ago

Closed 10 years ago

#3958 closed defect (fixed)

libvpx symbol check missing in configure, build-time error

Reported by: Stefano Sabatini Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: libvpx regression
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: yes

Description

I'm compiling FFmpeg with --enable-libvpx on Linux Mint, with libvpx version 1.2.0-2. Configuration succeds, but since commit

commit 04b0dda853198980a14158809e560ea9ca9d7e33
Author: Deb Mukherjee <debargha@google.com>
Date:   Sun Sep 14 07:43:27 2014 -0700

    avcodec/libvpxdec: Adds decode support for formats other than 420
    
    Handles decoding of new VP9 profiles 1-3 with different color sampling
    and bit-depths.
    
    For high bitdepth (profiles 2 and 3) support, we currently need to link
    with the highbitdepth branch of libvpx with --enable-experimental
    and --enable-vp9-high config options on. But eventually this branch will
    be merged into master, whereafter to enable high bitdepth
    support you will need to link with libvpx with configure option
    --enable-vp9-highbitdepth on.
    
    Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

I got the following building time failure:

CC	libavcodec/libvpxdec.o
libavcodec/libvpxdec.c: In function ‘set_pix_fmt’:
libavcodec/libvpxdec.c:71:14: error: ‘VPX_IMG_FMT_I422’ undeclared (first use in this function)
         case VPX_IMG_FMT_I422:
              ^
libavcodec/libvpxdec.c:71:14: note: each undeclared identifier is reported only once for each function it appears in
libavcodec/libvpxdec.c:74:14: error: ‘VPX_IMG_FMT_I444’ undeclared (first use in this function)
         case VPX_IMG_FMT_I444:
              ^
make: *** [libavcodec/libvpxdec.o] Error 1

This seems due to the reference to newly added symbols which are not included in my libvpx version. Adding a check in the FFmpeg configure script (ideally relying on some version check against the libvpx headers) should allow to fix the build failure.

Change History (5)

comment:1 by Carl Eugen Hoyos, 10 years ago

Keywords: configure removed
Status: newopen

This should be fixed in libavcodec imo, no need to break compilation with existing library installations for an experimental feature.

comment:2 by Benoit Fouet, 10 years ago

I don't know how this could be checked without using the configure script.
Here is an attempt to do it by adding an 'experimental' libvpx configuration:

diff --git a/configure b/configure
index 9b294f8..388e36a 100755
--- a/configure
+++ b/configure
@@ -1845,6 +1845,7 @@ CONFIG_EXTRA="
     iirfilter
     intrax8
     lgplv3
+    libvpx_experimental
     llauddsp
     llviddsp
     lpc
@@ -4863,7 +4864,9 @@ enabled libvpx            && {
     enabled libvpx_vp8_encoder && { check_lib2 "vpx/vpx_encoder.h vpx/vp8cx.h" "vpx_codec_enc_init_ver VP8E_SET_MAX_INTRA_BITRATE_PCT" -lvpx ||
                                     die "ERROR: libvpx encoder version must be >=0.9.7"; }
     enabled libvpx_vp9_decoder && { check_lib2 "vpx/vpx_decoder.h vpx/vp8dx.h" "vpx_codec_vp9_dx" -lvpx || disable libvpx_vp9_decoder; }
-    enabled libvpx_vp9_encoder && { check_lib2 "vpx/vpx_encoder.h vpx/vp8cx.h" "vpx_codec_vp9_cx VP9E_SET_AQ_MODE" -lvpx || disable libvpx_vp9_encoder; } }
+    enabled libvpx_vp9_encoder && { check_lib2 "vpx/vpx_encoder.h vpx/vp8cx.h" "vpx_codec_vp9_cx VP9E_SET_AQ_MODE" -lvpx || disable libvpx_vp9_encoder; }
+    check_lib2 vpx/vpx_image.h "VPX_IMG_FMT_I422 VPX_IMG_FMT_I444" && enable libvpx_experimental;
+}
 enabled libwavpack        && require libwavpack wavpack/wavpack.h WavpackOpenFileOutput  -lwavpack
 enabled libwebp           && require_pkg_config "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion
 enabled libx264           && require libx264 x264.h x264_encoder_encode -lx264 &&
diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
index 8312460..cbb6d0d 100644
--- a/libavcodec/libvpxdec.c
+++ b/libavcodec/libvpxdec.c
@@ -68,12 +68,14 @@ static int set_pix_fmt(AVCodecContext *avctx, struct vpx_image *img) {
         case VPX_IMG_FMT_I420:
             avctx->pix_fmt = AV_PIX_FMT_YUV420P;
             return 0;
+#if CONFIG_LIBVPX_EXPERIMENTAL
         case VPX_IMG_FMT_I422:
             avctx->pix_fmt = AV_PIX_FMT_YUV422P;
             return 0;
         case VPX_IMG_FMT_I444:
             avctx->pix_fmt = AV_PIX_FMT_YUV444P;
             return 0;
+#endif
 #ifdef VPX_IMG_FMT_HIGHBITDEPTH
         case VPX_IMG_FMT_I42016:
             if (img->bit_depth == 10) {

comment:3 by Carl Eugen Hoyos, 10 years ago

Sorry, I thought that there must be a define to check if the header contains the necessary definitions but this is apparently not the case;-(
I don't know if your patch isn't too invasive for this issue but since distributions exist with older headers (see mplayer-dev-eng today), I suggest this should be discussed on the mailing list.

in reply to:  2 comment:4 by jamal, 10 years ago

Replying to benoit:

I don't know how this could be checked without using the configure script.
Here is an attempt to do it by adding an 'experimental' libvpx configuration:

libvpx 1.2.0 is vp8 only.
Both VPX_IMG_FMT_I422 and VPX_IMG_FMT_I444 are available exclusively for vp9 starting with libvpx 1.3.0, the latest stable release, so it's not really "experimental".

A preprocessor check in libvpxdec.c should be enough to fix this. No need for extra configure checks.

comment:5 by jamal, 10 years ago

Resolution: fixed
Status: openclosed
Note: See TracTickets for help on using tickets.