Closed Bug 1284803 Opened 8 years ago Closed 8 years ago

Update libyuv to rev 1602

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox50 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

(Blocks 1 open bug)

Details

Attachments

(12 files, 2 obsolete files)

patch part 1 - Update libyuv to rev 1602
2.88 MB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 2 - Update moz.build for fixing build failure
590 bytes, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 3 - Change basic_types.h for fixing build failure
1.20 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 4 - Change libyuv.gyp for fixing build failure
1.37 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 5 - Enable JPEG
1.39 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 6- Suppress MJPEG fprintf() warnings in libyuv
2.35 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1
1.45 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 8 - Disable AVX2 asm if the compiler/assembler don't support it
3.01 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 9 - Make sure NEON ifdefs match
1.85 KB, patch
jesup
: review+
Details | Diff | Splinter Review
patch part 10 - Add toleranes to testColorConversions()
7.53 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
patch part 11 - Fix build failure of Android 4.2 x86 opt
1.64 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
patch part 12 - Update web-platform-tests webvtt ini
10.69 KB, patch
jgraham
: review+
Details | Diff | Splinter Review
libyuv is not updated since Bug 880419. It is rev 971 and it has a bug like Bug 1282711. It is better to update to its upstream.
Assignee: nobody → sotaro.ikeda.g
Blocks: libyuv-updates
Blocks: unaccel-video
libyuv code is very different between rev 971 and rev 1602. Then at first, I am going to replace libyuv to new one and then create patches to re-modify libyuv for gecko.
For addressing function definition conflict.
Attachment #8768352 - Attachment description: patch part 3 - Update basic_types.h → patch part 3 - Change basic_types.h
Attachment #8768352 - Attachment description: patch part 3 - Change basic_types.h → patch part 3 - Change basic_types.h for fixing build failure
Attachment #8768351 - Attachment description: patch part 2 - Update moz.build → patch part 2 - Update moz.build for fixing build failure
Attachment #8768354 - Attachment is obsolete: true
Attached patch patch part 5 - Enable JPEG — — Splinter Review
See Also: → 880419
Attachment #8768635 - Attachment description: patch - Suppress MJPEG fprintf() warnings in libyuv → patch part 6- Suppress MJPEG fprintf() warnings in libyuv
Attachment #8768650 - Attachment description: patch part 7 - Add yuv_disable_asm → patch part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1
https://treeherder.mozilla.org/#/jobs?repo=try&revision=80b902e05f31 test failures of testColorConversions() seemed to be addressed.
(In reply to Sotaro Ikeda [:sotaro] from comment #14) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=80b902e05f31 Build failure of Android 4.2 x86 opt was happened at I422AlphaToARGBRow_SSSE3() and I411ToARGBRow_SSSE3(). They fails to build on gcc/clang 32 bit with fpic and framepointer. It is written in row.h. It is written like the following. ------------------------------------ // The following functions fail on gcc/clang 32 bit with fpic and framepointer. // caveat: clangcl uses row_win.cc which works. #if defined(NDEBUG) || !(defined(_DEBUG) && defined(__i386__)) || \ !defined(__i386__) || defined(_MSC_VER) // TODO(fbarchard): fix build error on x86 debug // https://code.google.com/p/libyuv/issues/detail?id=524 #define HAS_I411TOARGBROW_SSSE3 // TODO(fbarchard): fix build error on android_full_debug=1 // https://code.google.com/p/libyuv/issues/detail?id=517 #define HAS_I422ALPHATOARGBROW_SSSE3 #endif #endif
The build failure has triggered by "ac_add_options --enable-profiling".
Comment on attachment 8768349 [details] [diff] [review] patch part 1 - Update libyuv to rev 1602 :jesup, can you feedback to the patches?
Attachment #8768349 - Flags: feedback?(rjesup)
Attachment #8769058 - Attachment is obsolete: true
Attachment #8768349 - Flags: feedback?(rjesup) → review?(rjesup)
Attachment #8768351 - Flags: review?(rjesup)
Attachment #8768352 - Flags: review?(rjesup)
Attachment #8768611 - Flags: review?(rjesup)
Attachment #8768621 - Flags: review?(rjesup)
Attachment #8768635 - Flags: review?(rjesup)
Attachment #8768650 - Flags: review?(rjesup)
Attachment #8768652 - Flags: review?(rjesup)
Attachment #8768656 - Flags: review?(rjesup)
Comment on attachment 8768349 [details] [diff] [review] patch part 1 - Update libyuv to rev 1602 Review of attachment 8768349 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Sotaro - if I recall, this also picks up some win64 improvements among many other things. Question: to ease future imports, shall we set up an update.sh similar to libopus/libvpx/etc, with a copy of each applied patch? Otherwise someone has to come find this bug (and all bugs applied against it later), and reapply the patches (and avoid missing any), or use a single "rollup" patch which loses some of the history of what change was done when/for-what-reason.
Attachment #8768349 - Flags: review?(rjesup) → review+
Attachment #8768351 - Flags: review?(rjesup) → review+
Comment on attachment 8768352 [details] [diff] [review] patch part 3 - Change basic_types.h for fixing build failure Review of attachment 8768352 [details] [diff] [review]: ----------------------------------------------------------------- I don't love that we still have to do this, but ok.
Attachment #8768352 - Flags: review?(rjesup) → review+
Attachment #8768611 - Flags: review?(rjesup) → review+
Attachment #8768621 - Flags: review?(rjesup) → review+
Attachment #8768635 - Flags: review?(rjesup) → review+
Attachment #8768650 - Flags: review?(rjesup) → review+
Attachment #8768652 - Flags: review?(rjesup) → review+
Attachment #8768656 - Flags: review?(rjesup) → review+
(In reply to Randell Jesup [:jesup] from comment #23) > Question: to ease future imports, shall we set up an update.sh similar to > libopus/libvpx/etc, with a copy of each applied patch? Otherwise someone > has to come find this bug (and all bugs applied against it later), and > reapply the patches (and avoid missing any), or use a single "rollup" patch > which loses some of the history of what change was done when/for-what-reason. It seems depends on a personal preference. libyuv seems to necessitate a bit different patches depends on a revision. Then, for the meantime, it seems better to come to "libyuv update" meta bug, and reapply the patches. I created Bug 1284800 as meta bug of libyuv update. In future, we could move to similar to libopus/libvpx/etc, I think.
Attachment #8768765 - Flags: review?(jmuizelaar)
Attachment #8769041 - Flags: review?(jmuizelaar)
Attachment #8769524 - Flags: review?(james)
Attachment #8769524 - Flags: review?(james) → review+
Attachment #8768765 - Flags: review?(jmuizelaar) → review+
Attachment #8769041 - Flags: review?(jmuizelaar) → review+
Pushed by sikeda@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/92c15211f59b part 1 - Update libyuv to rev 1602 r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/fb3d8468b758 part 2 - Update moz.build for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/52bd1efc6c1f part 3 - Change basic_types.h for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/109f79e577a0 part 4 - Change libyuv.gyp for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/6eb7bce6f4c1 part 5 - Enable JPEG r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/b23773965e94 part 6- Suppress MJPEG fprintf() warnings in libyuv r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/3b11ba39748c part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1 r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/9ea43addc3af part 8 - Disable AVX2 asm if the compiler/assembler don't support it r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/28e0ca5cb68e part 9 - Make sure NEON ifdefs match r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/6bacfaadbc29 part 10 - Add toleranes to testColorConversions() r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/08d2e46308fd part 11 - Fix build failure of Android 4.2 x86 opt r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/80468414501e part 12 - Update web-platform-tests webvtt ini r=jgraham
Sotaro - did you try some webrtc calls (even just https://mozilla.github.io/webrtc-landing/pc_test.html)?
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Randell Jesup [:jesup] from comment #28) > Sotaro - did you try some webrtc calls (even just > https://mozilla.github.io/webrtc-landing/pc_test.html)? With the patch, the above webtrc call also worked on my laptop. Before check-in, I simply tested webrtc at https://webrtc.github.io/samples/.
Flags: needinfo?(sotaro.ikeda.g)
Starting with the first push after this landed https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=0d582c2398722ced0d351cf330537a050a58ca5d , the Wr tests pass randomly /webvtt/rendering/cues-with-video/processing-model/align_start.html Please check that this is caused by the change and update the expected test results.
Flags: needinfo?(sotaro.ikeda.g)
Backing this out for those Wr failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6428db384f6
Status: RESOLVED → REOPENED
status-firefox50: fixed → affected
Resolution: FIXED → ---
Target Milestone: mozilla50 → ---
(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #31) > Starting with the first push after this landed > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > inbound&revision=0d582c2398722ced0d351cf330537a050a58ca5d , the Wr tests > pass randomly > /webvtt/rendering/cues-with-video/processing-model/align_start.html Please > check that this is caused by the change and update the expected test results. The patches just change YUV-RGB color conversion, it does not affect to WebVTT without a way of cpu usage. WebVTT does not implement ::cue pseudo-element yet, therefore, align_start.html test should always fail. It seems like WebVTT's problem. It might better to disable the related tests since ::cue pseudo-element is not implemented yet.
Flags: needinfo?(sotaro.ikeda.g)
> The patches just change YUV-RGB color conversion, it does not affect to > WebVTT without a way of cpu usage. WebVTT does not implement ::cue > pseudo-element yet, therefore, align_start.html test should always fail. Bug 865395 is for "::cue pseudo-element".
See Also: → vtt-css-extensions
Depends on: 1288648
(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #31) > Starting with the first push after this landed > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > inbound&revision=0d582c2398722ced0d351cf330537a050a58ca5d , the Wr tests > pass randomly > /webvtt/rendering/cues-with-video/processing-model/align_start.html Please > check that this is caused by the change and update the expected test results. Disabled tests in Bug 1288648.
Pushed by sikeda@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/65ee637b7e20 part 1 - Update libyuv to rev 1602 r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/612297895009 part 2 - Update moz.build for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/ccf9a31fe40d part 3 - Change basic_types.h for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/78a10fc91b52 part 4 - Change libyuv.gyp for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/ac2f07864fb1 part 5 - Enable JPEG r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/6dbd0dee3572 part 6- Suppress MJPEG fprintf() warnings in libyuv r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/f7f503edb55b part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1 r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/24ebb285aff3 part 8 - Disable AVX2 asm if the compiler/assembler don't support it r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/faf44eac1bb7 part 9 - Make sure NEON ifdefs match r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/65e1219ade06 part 10 - Add toleranes to testColorConversions() r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/d1cbc5eeb8c8 part 11 - Fix build failure of Android 4.2 x86 opt r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/92ba454cff2a part 12 - Update web-platform-tests webvtt ini r=jgraham
Sorry had to back your push out due to build bustage, e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=32581480&repo=mozilla-inbound#L2356
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Iris Hsiao [:ihsiao] from comment #37) > Sorry had to back your push out due to build bustage, e.g. > https://treeherder.mozilla.org/logviewer.html#?job_id=32581480&repo=mozilla- > inbound#L2356 Sorry, my check-in was bad :(
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Sotaro Ikeda [:sotaro] from comment #39) > > Sorry, my check-in was bad :( I forgot to add new files.
Pushed by sikeda@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/30ea7af8393e part 1 - Update libyuv to rev 1602 r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/8fc97d9901f2 part 2 - Update moz.build for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/fa3451c1f44b part 3 - Change basic_types.h for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/6e7013ad8460 part 4 - Change libyuv.gyp for fixing build failure r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/8f4c3438f844 part 5 - Enable JPEG r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/df6e1dfde6e3 part 6- Suppress MJPEG fprintf() warnings in libyuv r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/9344b69e9e32 part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1 r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/e0593340378d part 8 - Disable AVX2 asm if the compiler/assembler don't support it r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/b953bf23cde2 part 9 - Make sure NEON ifdefs match r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/3606c077fa9f part 10 - Add toleranes to testColorConversions() r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/d4429adcef8b part 11 - Fix build failure of Android 4.2 x86 opt r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/2fc1eb070c2a part 12 - Update web-platform-tests webvtt ini r=jgraham
as a note, this patch had a performance improvement on windows 8 for the video compositor test in talos: https://treeherder.mozilla.org/perf.html#/alerts?id=2066 the backout was a regression, and the relanding was the above improvement :)
Blocks: 1275441
Depends on: 1308471
Depends on: 1320452
Depends on: 1329383
Depends on: 1304330
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: