Closed
Bug 1151175
Opened 10 years ago
Closed 9 years ago
Update libvpx to 1.4.0
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: ionnv, Assigned: j)
References
()
Details
Attachments
(2 files, 5 obsolete files)
(deleted),
patch
|
rillian
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
rillian
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
https://groups.google.com/a/webmproject.org/forum/#!topic/codec-devel/2zYWenmdUM8
https://code.google.com/p/webm/source/browse/CHANGELOG?repo=libvpx
"libvpx is long overdue for a release. We have made 1.4.0 available for download and in the indianrunnerduck branch. Much like the Indian Runner Duck, the theme of this release is less bugs, less bloat and more speed."
Updated•10 years ago
|
QA Contact: giles
Updated•10 years ago
|
Assignee: nobody → giles
QA Contact: giles
Assignee | ||
Comment 1•9 years ago
|
||
- Update update.py to to work with libvpx 1.4
- Remove asm offsets from Makefile.in since they no longer exist
- update/drop patches
Assignee: giles → j
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8623013 -
Flags: review?(giles)
Assignee | ||
Comment 2•9 years ago
|
||
update libvpx to 1.4.0 (c74bf6d889992c3cabe017ec353ca85c323107cd)
Attachment #8623015 -
Flags: review?(giles)
Comment 3•9 years ago
|
||
Comment on attachment 8623013 [details] [diff] [review]
Bug1151175_update_update.py.patch
Review of attachment 8623013 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for taking this on. I'm surprised you can drop arm asm conversion entirely? Let me know how that works and ask for review again, please.
::: media/libvpx/Makefile.in
@@ -20,5 @@
> -
> -
> -ifdef VPX_AS_CONVERSION
> -# The ARM asm is written in ARM RVCT syntax, but we actually build it with
> -# gas using GNU syntax. Add some rules to perform the conversion.
What about the vp9 neon asm?
::: media/libvpx/msvc2015.patch
@@ +9,2 @@
> +# if _MSC_VER < 1900
> + # define snprintf _snprintf
This should still have a two-space indent, no?
Attachment #8623013 -
Flags: review?(giles)
Comment 4•9 years ago
|
||
On Mac:
0:39.81 Undefined symbols for architecture x86_64:
0:39.81 "_CONVERT_TO_SHORTPTR", referenced from:
0:39.81 _vp9_highbd_get16x16var_sse2 in vp9_highbd_variance_sse2.o
0:39.81 _vp9_highbd_10_get16x16var_sse2 in vp9_highbd_variance_sse2.o
0:39.81 _vp9_highbd_12_get16x16var_sse2 in vp9_highbd_variance_sse2.o
0:39.81 _vp9_highbd_get8x8var_sse2 in vp9_highbd_variance_sse2.o
0:39.81 _vp9_highbd_10_get8x8var_sse2 in vp9_highbd_variance_sse2.o
0:39.81 _vp9_highbd_12_get8x8var_sse2 in vp9_highbd_variance_sse2.o
0:39.81 _vp9_highbd_variance64x64_sse2 in vp9_highbd_variance_sse2.o
0:39.81 ...
0:39.81 "_total_adj_strong_thresh", referenced from:
0:39.81 _vp9_denoiser_NxM_sse2_small in vp9_denoiser_sse2.o
0:39.81 _vp9_denoiser_NxM_sse2_big in vp9_denoiser_sse2.o
0:39.81 ld: symbol(s) not found for architecture x86_64
0:39.81 clang: error: linker command failed with exit code 1 (use -v to see invocation)
0:39.81 make[6]: *** [XUL] Error 1
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Ralph Giles (:rillian) from comment #3)
> Comment on attachment 8623013 [details] [diff] [review]
> Bug1151175_update_update.py.patch
>
> Review of attachment 8623013 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Thanks for taking this on. I'm surprised you can drop arm asm conversion
> entirely? Let me know how that works and ask for review again, please.
>
> ::: media/libvpx/Makefile.in
> @@ -20,5 @@
> > -
> > -
> > -ifdef VPX_AS_CONVERSION
> > -# The ARM asm is written in ARM RVCT syntax, but we actually build it with
> > -# gas using GNU syntax. Add some rules to perform the conversion.
>
> What about the vp9 neon asm?
ups, that was indeed to much, will add back in.
> ::: media/libvpx/msvc2015.patch
> @@ +9,2 @@
> > +# if _MSC_VER < 1900
> > + # define snprintf _snprintf
>
> This should still have a two-space indent, no?
ok
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Ralph Giles (:rillian) from comment #4)
> On Mac:
>
> 0:39.81 Undefined symbols for architecture x86_64:
> 0:39.81 "_CONVERT_TO_SHORTPTR", referenced from:
> 0:39.81 _vp9_highbd_get16x16var_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 _vp9_highbd_10_get16x16var_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 _vp9_highbd_12_get16x16var_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 _vp9_highbd_get8x8var_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 _vp9_highbd_10_get8x8var_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 _vp9_highbd_12_get8x8var_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 _vp9_highbd_variance64x64_sse2 in vp9_highbd_variance_sse2.o
> 0:39.81 ...
> 0:39.81 "_total_adj_strong_thresh", referenced from:
> 0:39.81 _vp9_denoiser_NxM_sse2_small in vp9_denoiser_sse2.o
> 0:39.81 _vp9_denoiser_NxM_sse2_big in vp9_denoiser_sse2.o
> 0:39.81 ld: symbol(s) not found for architecture x86_64
> 0:39.81 clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> 0:39.81 make[6]: *** [XUL] Error 1
fixed my universal build setup and fixed 32bit build.
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8623013 -
Attachment is obsolete: true
Attachment #8623599 -
Flags: review?(giles)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8623015 -
Attachment is obsolete: true
Attachment #8623015 -
Flags: review?(giles)
Attachment #8623600 -
Flags: review?(giles)
Assignee | ||
Comment 9•9 years ago
|
||
replace with hg version of patch
Attachment #8623599 -
Attachment is obsolete: true
Attachment #8623599 -
Flags: review?(giles)
Attachment #8623601 -
Flags: review?(giles)
Comment 10•9 years ago
|
||
Comment on attachment 8623601 [details] [diff] [review]
Bug1151175_update_update.py.patch
Thanks, changes look good. Still some issues:
undefined reference to `CONVERT_TO_SHORTPTR' on linux64
undefined reference to `total_adj_strong_thresh' on linux
moz.build parse errors on windows
multiple definitions of vp9 neon asm functions on android
See build reports at https://treeherder.mozilla.org/#/jobs?repo=try&revision=56db250ea91a
Attachment #8623601 -
Flags: review?(giles) → review-
Assignee | ||
Comment 11•9 years ago
|
||
yes found 2 other issue
- with neon if both intrinsics and asm exist (now only using asm)
- and some unused options (VP9_TEMPORAL_DENOISING and VP9_HIGHBITDEPTH)
waiting for my local windows build and will post an updated patch once that is done.
Assignee | ||
Comment 12•9 years ago
|
||
new patch fixing some more issue:
- with neon if both intrinsics and asm exist (now only using asm)
- and some unused options (VP9_TEMPORAL_DENOISING and VP9_HIGHBITDEPTH)
now tested build on
- linux x86_64
- osx x86_64 / i386
- osx cross compile android
compiles on win32 (VC2013) but my vm runs out of memory to link. might be faster to test on tryserver.
Attachment #8623601 -
Attachment is obsolete: true
Attachment #8623911 -
Flags: review?(giles)
Assignee | ||
Comment 13•9 years ago
|
||
and the corresponding libvpx patch
Attachment #8623600 -
Attachment is obsolete: true
Attachment #8623600 -
Flags: review?(giles)
Attachment #8623912 -
Flags: review?(giles)
Comment 14•9 years ago
|
||
try is closed. I'll push again when it opens.
Comment 15•9 years ago
|
||
Comment 16•9 years ago
|
||
Comment on attachment 8623911 [details] [diff] [review]
Bug1151175_update_update.py.patch
This one looks good, except I think we need to clobber the build to work around mach not handling vp9_thread.c moving locations. I've pushed again to verify. If that makes it green, we should be ready to land with the clobber change folded in.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6048116a5b0b
Attachment #8623911 -
Flags: review?(giles) → review+
Comment 17•9 years ago
|
||
Comment on attachment 8623912 [details] [diff] [review]
Bug1151175_update_libvpx.patch
r=me with CLOBBER updated and a green try push.
Attachment #8623912 -
Flags: review?(giles) → review+
Comment 18•9 years ago
|
||
Try looks green.
Comment 19•9 years ago
|
||
Comment 20•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/13d883caaf74
https://hg.mozilla.org/mozilla-central/rev/de1ddaec57b7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 21•9 years ago
|
||
Comment on attachment 8623912 [details] [diff] [review]
Bug1151175_update_libvpx.patch
Requesting aurora uplift for both patches on this bug. This gets the new, stable upstream code to users sooner, and we've had one report of a rendering regression fixed by this update in bug 1175220.
Approval Request Comment
[Feature/regressing bug #]: 1148639
[User impact if declined]: Rendering issues, likely security issues, with webm video.
[Describe test coverage new/current, TreeHerder]: Landed on m-c, green on media tests.
[Risks and why]: Risk is low. We're moving from a snapshot to a released version of a third-party library which has been out for a couple of months.
[String/UUID change made/needed]: None.
Attachment #8623912 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox40:
--- → affected
Comment 22•9 years ago
|
||
Comment on attachment 8623912 [details] [diff] [review]
Bug1151175_update_libvpx.patch
Landed a few days ago in m-c without obvious regressions. As a Linux packager, I am familiar with update of libraries and I am not concerned about that. taking it.
Attachment #8623912 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 23•9 years ago
|
||
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•