After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 739476 - vpx: fails to build against libvpx from git
vpx: fails to build against libvpx from git
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.4.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-01 01:51 UTC by Basil Mohamed Gohar
Modified: 2014-11-12 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vpx: check image abi version prior to use compatibility defines (990 bytes, patch)
2014-11-01 12:23 UTC, Aurélien Zanelli
needs-work Details | Review
vpx: remove compatibility defines (1.04 KB, patch)
2014-11-01 15:27 UTC, Aurélien Zanelli
committed Details | Review
vpx: mark arnr-type properties as deprecated and set them to a no-op (4.32 KB, patch)
2014-11-01 21:46 UTC, Aurélien Zanelli
none Details | Review
vpx: mark arnr-type properties as deprecated and set them to no-op (4.34 KB, patch)
2014-11-01 23:32 UTC, Aurélien Zanelli
committed Details | Review

Description Basil Mohamed Gohar 2014-11-01 01:51:48 UTC
I'm attempting to build gstreamer from git source, and gst-plugins-good is failing to build with an error in the code that calls libvpx.  The error output in question is:

================================================================================
make[3]: Entering directory `/home/bgohar/src/gst/gst-plugins-good/ext/vpx'
  CC       libgstvpx_la-gstvp8dec.lo
In file included from gstvp8dec.c:49:0:
gstvp8dec.c: In function 'gst_vp8_dec_handle_frame':
gstvp8utils.h:30:26: error: 'IMG_FMT_I420' undeclared (first use in this function)
 #define VPX_IMG_FMT_I420 IMG_FMT_I420
                          ^
gstvp8dec.c:542:21: note: in expansion of macro 'VPX_IMG_FMT_I420'
     if (img->fmt != VPX_IMG_FMT_I420) {
                     ^
gstvp8utils.h:30:26: note: each undeclared identifier is reported only once for each function it appears in
 #define VPX_IMG_FMT_I420 IMG_FMT_I420
                          ^
gstvp8dec.c:542:21: note: in expansion of macro 'VPX_IMG_FMT_I420'
     if (img->fmt != VPX_IMG_FMT_I420) {
                     ^
make[3]: *** [libgstvpx_la-gstvp8dec.lo] Error 1
make[3]: Leaving directory `/home/bgohar/src/gst/gst-plugins-good/ext/vpx'
make[2]: *** [vpx] Error 2
make[2]: Leaving directory `/home/bgohar/src/gst/gst-plugins-good/ext'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/bgohar/src/gst/gst-plugins-good'
make: *** [all] Error 2
================================================================================

A full discussion on the webm-discuss mailing list where the issue was brought up to the libvpx devs and they suggested what might be the problem can be found here:

https://groups.google.com/a/webmproject.org/forum/#!topic/webm-discuss/cMtZfP-02pk
Comment 1 Aurélien Zanelli 2014-11-01 11:44:59 UTC
Hi, thanks for reporting the bug.

Since ABI version of libvpx has changed, I think we can check it to determine if we should redefine VPX_* or not.
I am writing a patch.
Comment 2 Aurélien Zanelli 2014-11-01 12:23:14 UTC
Created attachment 289776 [details] [review]
vpx: check image abi version prior to use compatibility defines

Proposal patch to fix compatibility defines with recent version of libvpx.

However, vp8enc and vp9enc currently exports a property "PROP_ARNR_TYPE" which is deprecated leading the compilation to fail. 

For now we can either remove this property from vpxenc or try to disable the deprecation warning. What do you prefer ?
Comment 3 Basil Mohamed Gohar 2014-11-01 12:49:31 UTC
I'm asking upstream and I'll update here when I have an answer.
Comment 4 Tim-Philipp Müller 2014-11-01 13:52:16 UTC
We can't remove the property, it's part of the element's ABI. But we can mark it as deprecated in the property description.
Comment 5 Aurélien Zanelli 2014-11-01 15:06:09 UTC
(In reply to comment #3)
> I'm asking upstream and I'll update here when I have an answer.

Question was for GStreamer developers. :)

(In reply to comment #4)
> We can't remove the property, it's part of the element's ABI. But we can mark
> it as deprecated in the property description.

It makes sense to keep the property and mark it as deprecated, but could the property do nothing ?
Currently it doesn't compile because of the -Werror=deprecated-declarations and I didn't found a nice way to disable libvpx deprecation warning except redefining macros they used for deprecation (DEPRECATED and DECLSPEC_DEPRECATED).
Comment 6 Tim-Philipp Müller 2014-11-01 15:17:41 UTC
Comment on attachment 289776 [details] [review]
vpx: check image abi version prior to use compatibility defines

We require libvpx >= 1.1.0, so VPX_IMG_FMT_I420 is always available (as enum, not define).

So I think we shuold just remove those compatibility defines entirely.
Comment 7 Aurélien Zanelli 2014-11-01 15:27:37 UTC
Created attachment 289789 [details] [review]
vpx: remove compatibility defines

As Tim said, we are guaranteed to have these enums and defines in libvpx.
Comment 8 Tim-Philipp Müller 2014-11-01 15:45:19 UTC
Comment on attachment 289789 [details] [review]
vpx: remove compatibility defines

Pushed, thanks:

commit 130873c8fdb2a088fdff9b0b4592df728673c931
Author: Aurélien Zanelli <aurelien.zanelli@darkosphere.fr>
Date:   Sat Nov 1 12:18:02 2014 +0100

    vpx: remove compatibility defines
    
    We are guaranteed to have VPX_IMG_FMT_I420, VPX_PLANE_Y,
    VPX_PLANE_U and VPX_PLANE_V as we require libvpx > 1.1.0.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=739476

Keeping bug open for the other issue with the property.
Comment 9 Sebastian Dröge (slomo) 2014-11-01 17:49:40 UTC
(In reply to comment #4)
> We can't remove the property, it's part of the element's ABI. But we can mark
> it as deprecated in the property description.

And we can also change the behaviour of the property to a no-op (and print a g_warning() if used or something) to prevent usage of deprecated libvpx API.
Comment 10 Aurélien Zanelli 2014-11-01 21:46:42 UTC
Created attachment 289814 [details] [review]
vpx: mark arnr-type properties as deprecated and set them to a no-op

Patch proposal to mark the ARNR type properties as deprecated and do nothing except display a warning.
Comment 11 Aurélien Zanelli 2014-11-01 23:32:47 UTC
Created attachment 289817 [details] [review]
vpx: mark arnr-type properties as deprecated and set them to no-op

warning messages are now the same.
Comment 12 Basil Mohamed Gohar 2014-11-02 05:28:01 UTC
I apologize if this is derailing existing work, but I just wanted to report that I'm now getting a different error when I attempt to build from gst-plugins-good git:

================================================================================
make -C vpx
make[3]: Entering directory `/home/bgohar/src/gst/gst-plugins-good/ext/vpx'
  CC       libgstvpx_la-gstvp8dec.lo
  CC       libgstvpx_la-gstvp8enc.lo
gstvp8enc.c: In function 'gst_vp8_enc_set_property':
gstvp8enc.c:1131:9: error: 'vpx_codec_control_VP8E_SET_ARNR_TYPE' is deprecated (declared at /usr/local/include/vpx/vp8cx.h:379) [-Werror=deprecated-declarations]
         status = vpx_codec_control (&gst_vp8_enc->encoder, VP8E_SET_ARNR_TYPE,
         ^
gstvp8enc.c: In function 'gst_vp8_enc_set_format':
gstvp8enc.c:1669:3: error: 'vpx_codec_control_VP8E_SET_ARNR_TYPE' is deprecated (declared at /usr/local/include/vpx/vp8cx.h:379) [-Werror=deprecated-declarations]
   status = vpx_codec_control (&encoder->encoder, VP8E_SET_ARNR_TYPE,
   ^
cc1: all warnings being treated as errors
make[3]: *** [libgstvpx_la-gstvp8enc.lo] Error 1
make[3]: Leaving directory `/home/bgohar/src/gst/gst-plugins-good/ext/vpx'
make[2]: *** [vpx] Error 2
make[2]: Leaving directory `/home/bgohar/src/gst/gst-plugins-good/ext'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/bgohar/src/gst/gst-plugins-good'
make: *** [all] Error 2

================================================================================
Comment 13 Basil Mohamed Gohar 2014-11-02 05:29:55 UTC
I apologize if that was premature.  I just realized that is most likely related to the patch above, noting the similarity in the error message and content of the message.  I will refrain from posting build attempts until I see something a clearer message that the build should work.
Comment 14 Basil Mohamed Gohar 2014-11-10 15:11:32 UTC
I'd just like to tickle this and ask if there's anything I can do to help.  Anything you need from the upstream team?  I first notified them about this issue, and they're interested in getting it resolved as well.  There's also some murmurs of a new release down the line, not sure if that's something interest to the maintainers of this plugin.
Comment 15 Sebastian Dröge (slomo) 2014-11-10 15:15:04 UTC
Thanks, but there was nothing missing other than somebody merging this patch :)


commit d0e8a385e0a6089ea5a52681927dda52d332c7d9
Author: Aurélien Zanelli <aurelien.zanelli@darkosphere.fr>
Date:   Sat Nov 1 22:39:41 2014 +0100

    vpx: mark arnr-type properties as deprecated and set them to no-op
    
    ARNR type control in libvpx has been deprecated so this commit mark the
    vp8enc and vp9enc associated properties as deprecated and change their
    behavior to just display a warning message.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=739476
Comment 16 Basil Mohamed Gohar 2014-11-10 19:19:14 UTC
Awesome!  Thank you so much.  I just pulled down the latest git (after my last comment) and rebuilt, and it made it through.  Not only that, but I can play WebM videos again in Totem. :)