Closed Bug 945859 Opened 6 years ago Closed 6 years ago

build with --disable-webrtc fails linking libvpx

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox28 --- fixed

People

(Reporter: j, Assigned: j)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

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.
Attachment #8341870 - Flags: review+
with updated commit message.
Attachment #8341870 - Attachment is obsolete: true
Attachment #8341884 - Flags: review?(giles)
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.
Attachment #8341884 - Flags: review?(ted)
Attachment #8341884 - Flags: review?(giles)
Attachment #8341884 - Flags: 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
Attachment #8341884 - Flags: feedback+
Duplicate of this bug: 945917
Blocks: 763495
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..
Blocks: 947160
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!
Attachment #8341884 - Flags: review?(ted) → review+
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. :)
https://hg.mozilla.org/mozilla-central/rev/01762a3bb7fb
Assignee: nobody → j
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.