Closed Bug 1178215 Opened 9 years ago Closed 9 years ago

Update libvpx to more recent git version (1.4.x)

Categories

(Core :: Audio/Video, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox39 --- wontfix
firefox40 --- verified
firefox41 --- fixed
firefox42 --- fixed

People

(Reporter: j, Assigned: j)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

libvpx got quite some updates in git. Would be good to update to a more recent version. Chromium uses e67d45d4ce92468ba193288b59093fef0a502662 with one patch.
Attachment #8627129 - Flags: review?(giles)
Attached patch Bug1178215_update_libvpx.patch (obsolete) — Splinter Review
Attachment #8627130 - Flags: review?(giles)
Comment on attachment 8627129 [details] [diff] [review]
Bug1178215_update_update.py.patch

Review of attachment 8627129 [details] [diff] [review]:
-----------------------------------------------------------------

Please split the update.py, patch and README_MOZILLA changes into a separate commit from the config updates which result from running the new update.py, so we have a clear point of reproducibility. r=me with that change.
Attachment #8627129 - Flags: review?(giles) → review+
Comment on attachment 8627130 [details] [diff] [review]
Bug1178215_update_libvpx.patch

Review of attachment 8627130 [details] [diff] [review]:
-----------------------------------------------------------------

Likewise, r=me with the update.py results split into either this commit, or a third patch.

Thanks for the update.
Attachment #8627130 - Flags: review?(giles) → review+
Attachment #8627129 - Attachment is obsolete: true
Attachment #8627130 - Attachment is obsolete: true
now fully untangled, time for a try push?
Flags: needinfo?(giles)
Blocks: 1178831
Comment on attachment 8627371 [details] [diff] [review]
Bug1178215_update_update.py.patch

Review of attachment 8627371 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +5592,5 @@
>        AC_MSG_WARN([No assembler or assembly support for libvpx. Using unoptimized C routines.])
>      fi
> +
> +    dnl native libvpx no longer has vpx_mem_set_functions
> +    AC_DEFINE(MOZ_VPX_NO_MEM_REPORTING)

We should patch this back in. I filed bug 1178831 for that.
Attachment #8627371 - Flags: review+
Attachment #8627373 - Flags: review+
*** No rule to make target `/home/worker/workspace/gecko/media/libvpx/vp8/common/x86/variance_mmx.c', needed by `variance_mmx.o'.  Stop.

Pushed again with CLOBBER update. Try used to auto-clobber, but maybe that's changed? Looks like the familiar mach bug with moved files.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce5c88684b39
Flags: needinfo?(giles)
Try looks good.
https://hg.mozilla.org/mozilla-central/rev/fa9904c62103
https://hg.mozilla.org/mozilla-central/rev/a5efc589a3d6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Target Milestone: mozilla41 → mozilla42
Comment on attachment 8627373 [details] [diff] [review]
Bug1178215_update_libvpx.patch

Requesting uplift of both patches for Aurora and Beta.

Approval Request Comment
[Feature/regressing bug #]: Bug 1151175
[User impact if declined]: We need to take this for security and stability.
[Describe test coverage new/current, TreeHerder]: Landed on m-c some weeks ago.
[Risks and why]: This is an update to the same third-party snapshot chrome is shipping. I don't expect any issues.
[String/UUID change made/needed]: None.
Flags: needinfo?(abillings)
Attachment #8627373 - Flags: approval-mozilla-beta?
Attachment #8627373 - Flags: approval-mozilla-aurora?
Depends on: 1176730
Comment on attachment 8627373 [details] [diff] [review]
Bug1178215_update_libvpx.patch

This update has been on m-c for a week. We're still early enough in Beta to take a bigger change like this.

Ralph - While you don't expect any issues, how will regressions due to this change appear in the browser if they are to appear? Or, what should we test for?

Beta+ Aurora+
Attachment #8627373 - Flags: approval-mozilla-beta?
Attachment #8627373 - Flags: approval-mozilla-beta+
Attachment #8627373 - Flags: approval-mozilla-aurora?
Attachment #8627373 - Flags: approval-mozilla-aurora+
Flags: needinfo?(abillings)
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #15)

> Ralph - While you don't expect any issues, how will regressions due to this
> change appear in the browser if they are to appear? Or, what should we test
> for?

Regressions would show primarily as video corruption or crashes in desktop-to-desktop webrtc (mozilla hello) sessions. Also the same for webm playback in the <video> element, but that's not broadly used in these firefox versions.
We have performed today regression testing using Firefox 40 Beta 3 (BuildID=20150709163524) and focused on Hello functionality and Video/Audio compatibility. There were no issues encountered related to video corruption for either Hello calls between different desktop machines, or Video playback. Detailed test results are available at: https://goo.gl/bWDsSd. 

Marking this as verified for Firefox 40 based on our testing results.
You need to log in before you can comment on or make changes to this bug.