Closed
Bug 750447
Opened 12 years ago
Closed 12 years ago
Builds broken after libvpx update
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: mpk, Assigned: derf)
References
Details
Attachments
(2 files, 1 obsolete file)
7.64 KB,
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
3.56 KB,
patch
|
cpearce
:
review+
gaston
:
feedback+
|
Details | Diff | Splinter Review |
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[5]: 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[5]: *** [postproc.o] Error 1 gmake[5]: 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.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → tterribe
Comment 1•12 years ago
|
||
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'
Comment 2•12 years ago
|
||
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)
Assignee | ||
Comment 3•12 years ago
|
||
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-
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #619949 -
Attachment is obsolete: true
Attachment #619959 -
Flags: review?(cpearce)
Updated•12 years ago
|
Attachment #619959 -
Flags: review?(cpearce) → review+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7a4373dd60ed
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 7•12 years ago
|
||
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 → ---
Comment 8•12 years ago
|
||
Forgot to add this was on OpenBSD/amd64, so not on powerpc like the last time...
Comment 9•12 years ago
|
||
And the new breakage is likely caused by bug #757637 / https://hg.mozilla.org/mozilla-central/rev/a9aeaa818e64 (wild guess)
Assignee | ||
Comment 10•12 years ago
|
||
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).
Comment 11•12 years ago
|
||
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)
Comment 12•12 years ago
|
||
(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 13•12 years ago
|
||
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..
Attachment #640206 -
Flags: review?(cpearce)
Attachment #640206 -
Flags: feedback+
Comment 14•12 years ago
|
||
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+
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f90923727eee
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: mozilla15 → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•