Closed Bug 750447 Opened 10 years ago Closed 10 years ago
Builds broken after libvpx update
The landing of the patch for bug 730907 unfortunately broke the builds on FreeBSD (and maybe other tier-2 and tier-3 platforms). The build log contains: [...] gmake: Entering directory `/work/fox/obj-x86_64-unknown-freebsd9.0/media/libvpx' [...] postproc.c /work/fox/media/libvpx/vp8/common/postproc.c:451:19: warning: explicitly assigning a variable of type 'int' to itself [-Wself-assign] for (next = next; next < 256; next++) ~~~~ ^ ~~~~ /work/fox/media/libvpx/vp8/common/postproc.c:869:67: error: no member named 'rtcd' in 'struct VP8Common' &cm->rtcd.... ~~ ^ /work/fox/media/libvpx/vp8/common/postproc.c:886:59: error: no member named 'rtcd' in 'struct VP8Common' &cm->rtcd.variance); ~~ ^ 1 warning and 2 errors generated. In the directory /work/fox/obj-x86_64-unknown-freebsd9.0/media/libvpx The following command failed to execute properly: clang -o postproc.o [...] /work/fox/media/libvpx/vp8/common/postproc.c gmake: *** [postproc.o] Error 1 gmake: Leaving directory `/work/fox/obj-x86_64-unknown-freebsd9.0/media/libvpx' [...] I suspect it broke because CONFIG_RUNTIME_CPU_DETECT is set to 0 for generic platforms. In /media/libvpx/vp8/common/onyxc_int.h rtcd is defined conditionally but in media/libvpx/vp8/common/postproc.c it is used unconditionally. Maybe there are other such occurrences throughout the code. Didn't find the time yet to dig deeper.
Same issue on OpenBSD/ppc with gcc : /home/landry/src/mozilla-central/media/libvpx/vp8/common/postproc.c: In function 'vp8_multiframe_quality_enhance': /home/landry/src/mozilla-central/media/libvpx/vp8/common/postproc.c:869: error: 'VP8_COMMON' has no member named 'rtcd' /home/landry/src/mozilla-central/media/libvpx/vp8/common/postproc.c:886: error: 'VP8_COMMON' has no member named 'rtcd'
Here's a tentative patch that seems to fix the issue here, at least build goes further than libvpx. Rationale is to pass NULL when CONFIG_RUNTIME_CPU_DETECT is 0, since VARIANCE_INVOKE() ignores first argument if CONFIG_RUNTIME_CPU_DETECT is 0 (see end of media/libvpx/vp8/common/variance.h)
Attachment #619949 - Flags: review?(tterribe)
Comment on attachment 619949 [details] [diff] [review] pass null for variance on plaforms where CONFIG_RUNTIME_CPU_DETECT is 0 Review of attachment 619949 [details] [diff] [review]: ----------------------------------------------------------------- The idea here is good, but it needs to be slotted into update.sh so this patch can be applied with the others we apply to libvpx. I'll post a patch that does that shortly. ::: media/libvpx/vp8/common/postproc.c @@ +805,5 @@ > } > > #if CONFIG_RUNTIME_CPU_DETECT > #define RTCD_VTABLE(oci) (&(oci)->rtcd.postproc) > +#define VARIANCE(oci) (&(oci)->rtcd.variance) I'd have preferred to use IF_RTCD-style macros for both postproc and variance, like the encoder does, but sadly they hard-code the variable name of the encoder struct (cpi). It's probably not worth the pain to do something cleaner here, sine all this code is gone from upstream, but I think this should at least have an RTCD in the name.
Attachment #619949 - Flags: review?(tterribe) → review-
Attachment #619959 - Flags: review?(cpearce) → review+
Target Milestone: --- → mozilla15
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
This build breakage came back (likely due to a recent commit to libvpx, or webrtc landing/enabling, didnt find it yet) : /home/landry/src/mozilla-central/media/libvpx/vp8/encoder/pickinter.c: In function 'pick_intra4x4block': /home/landry/src/mozilla-central/media/libvpx/vp8/encoder/pickinter.c:135: error: 'VP8_COMMON_RTCD' has no member named 'variance' Testing a quick fix - distortion = get_prediction_error(be, b, &rtcd->common->variance); + distortion = get_prediction_error(be, b, RTCD_VARIANCE(rtcd->common)); but i think variance-invoke.patch should be extended to also use RTCD_VARIANCE (or a variation of it) here.. Reopening this bug instead of creating a new one, but if preferred (and if it appears it's a different issue) i can create a separate one.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Forgot to add this was on OpenBSD/amd64, so not on powerpc like the last time...
And the new breakage is likely caused by bug #757637 / https://hg.mozilla.org/mozilla-central/rev/a9aeaa818e64 (wild guess)
Initial patch. Let me know if this fixes the problem for you. Try results still coming in: https://tbpl.mozilla.org/?tree=Try&rev=311c8352d807 But I don't expect any problems since this shouldn't change the post-preprocessing code on any platform on try. Speaking of, if your OpenBSD/amd64 build isn't using the assembler, it probably should be. Feel free to open a bug on that (though I can't fix it myself as I don't have any way to test).
webrtc was disabled on *bsd in #771981 so the failure temporarly disappeared, but i'll test your patch when trying to fix/enable webrtc on openbsd. I think amd64 uses the assembler (will have to check) , the original issue was on ppc (which doesnt use the assembler)
(In reply to Timothy B. Terriberry (:derf) from comment #10) > Speaking of, if your OpenBSD/amd64 build isn't using the assembler, it > probably should be. Feel free to open a bug on that (though I can't fix it > myself as I don't have any way to test). After looking more carefully (see http://mxr.mozilla.org/mozilla-central/source/configure.in#5440) , it seems that the assembler is used in libvpx only on Linux/SunOS/Darwin/WINNT for x86 and x86_64.VPX_X86_ASM is _not_ set on OpenBSD/amd64. I'll file another bug for that, but first i'll try your diff to see if it fixes the last build issue on platforms where CONFIG_RUNTIME_CPU_DETECT is 0.
Comment on attachment 640206 [details] [diff] [review] Wrap &rtcd->common->variance in IF_RTCD I can confirm that with this diff, libvpx builds fine when --enable-webrtc is passed to configure on OpenBSD/amd64 where CONFIG_RUNTIME_CPU_DETECT is 0. Builds fails later on in webrtc but that's bug 771981. (wondering why libvpx builds fine when webrtc is disabled...) Passing r? to cpearce..
Comment on attachment 640206 [details] [diff] [review] Wrap &rtcd->common->variance in IF_RTCD Review of attachment 640206 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #640206 - Flags: review?(cpearce) → review+
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: mozilla15 → mozilla17
You need to log in before you can comment on or make changes to this bug.