User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release) Build ID: 20131203030202 Steps to reproduce: libvpx fails to link if encoder is not enabled. This can be fixed by always building the encoder. This will fix --disable-webrtc builds.

Comment on attachment 8341870 [details] [diff] [review] 0001-Always-build-encoder-if-building-libvpx.patch Review of attachment 8341870 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the fix. Please put the 'steps to reproduce' in the commit message so we have the motivation in the changelog.

with updated commit message.

Comment on attachment 8341884 [details] [diff] [review] 0001-Always-build-encoder-if-building-libvpx.patch Review of attachment 8341884 [details] [diff] [review]: ----------------------------------------------------------------- Works for me. Passing to Ted for build peer review.

Comment on attachment 8341884 [details] [diff] [review] 0001-Always-build-encoder-if-building-libvpx.patch Fwiw, this also fixes the breakage for me as reported in the dupe #945917

If you're going to always build with the encoder enabled, please remove MOZ_VP8_ENCODER completely to save any future work/confusion.

I agree. The MOZ_VP8_ENCODER option was originally added to make it easy to disable if we ran into problems when encoding support was new, but I think we're long past that point now.

I have a patch ready to remove MOZ_VP8_ENCODER once Bug 918550 is merged.

This was just intended as a minimal fix to unbreak people's builds in the meantime.

Yes please, especially so that it can be fixed before the next uplift..

Comment on attachment 8341884 [details] [diff] [review] 0001-Always-build-encoder-if-building-libvpx.patch Review of attachment 8341884 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, didn't realize how trivial this was!

https://hg.mozilla.org/integration/mozilla-inbound/rev/01762a3bb7fb (sorry jan, you'll have to rebase 947160 on top but at least this wont migrate to aurora)

(In reply to Landry Breuil (:gaston) from comment #12) > https://hg.mozilla.org/integration/mozilla-inbound/rev/01762a3bb7fb (sorry > jan, you'll have to rebase 947160 on top but at least this wont migrate to > aurora) Bug 947160 is on top of this one so all fine.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #11) > Sorry, didn't realize how trivial this was! Thanks for the review. Somehow I thought you had a magic patch-length detector. Next time I'll say if it's a two-liner. :)