Update libyuv to rev 1602

RESOLVED FIXED in Firefox 50

Status

()

Core
Graphics
RESOLVED FIXED
2 years ago
11 months ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

(Depends on: 2 bugs, Blocks: 2 bugs)

unspecified
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(12 attachments, 2 obsolete attachments)

2.88 MB, patch
jesup
: review+
Details | Diff | Splinter Review
590 bytes, patch
jesup
: review+
Details | Diff | Splinter Review
1.20 KB, patch
jesup
: review+
Details | Diff | Splinter Review
1.37 KB, patch
jesup
: review+
Details | Diff | Splinter Review
1.39 KB, patch
jesup
: review+
Details | Diff | Splinter Review
2.35 KB, patch
jesup
: review+
Details | Diff | Splinter Review
1.45 KB, patch
jesup
: review+
Details | Diff | Splinter Review
3.01 KB, patch
jesup
: review+
Details | Diff | Splinter Review
1.85 KB, patch
jesup
: review+
Details | Diff | Splinter Review
7.53 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
1.64 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
10.69 KB, patch
jgraham
: review+
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
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)

Updated

2 years ago
Assignee: nobody → sotaro.ikeda.g
Blocks: 1284800
(Assignee)

Updated

2 years ago
Blocks: 1253062
(Assignee)

Comment 1

2 years ago
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.
(Assignee)

Comment 2

2 years ago
Created attachment 8768349 [details] [diff] [review]
patch part 1 - Update libyuv to rev 1602
(Assignee)

Comment 3

2 years ago
Created attachment 8768351 [details] [diff] [review]
patch part 2 - Update moz.build for fixing build failure

For addressing function definition conflict.
(Assignee)

Comment 4

2 years ago
Created attachment 8768352 [details] [diff] [review]
patch part 3 - Change basic_types.h  for fixing build failure
(Assignee)

Updated

2 years ago
Attachment #8768352 - Attachment description: patch part 3 - Update basic_types.h → patch part 3 - Change basic_types.h
(Assignee)

Comment 5

2 years ago
Created attachment 8768354 [details] [diff] [review]
patch part 4 - Change libyuv.gyp for fixing build failure
(Assignee)

Updated

2 years ago
Attachment #8768352 - Attachment description: patch part 3 - Change basic_types.h → patch part 3 - Change basic_types.h for fixing build failure
(Assignee)

Updated

2 years ago
Attachment #8768351 - Attachment description: patch part 2 - Update moz.build → patch part 2 - Update moz.build for fixing build failure
(Assignee)

Comment 6

2 years ago
Created attachment 8768611 [details] [diff] [review]
patch part 4 - Change libyuv.gyp for fixing build failure
Attachment #8768354 - Attachment is obsolete: true
(Assignee)

Comment 7

2 years ago
Created attachment 8768621 [details] [diff] [review]
patch part 5 - Enable JPEG
(Assignee)

Updated

2 years ago
See Also: → bug 880419
(Assignee)

Comment 8

2 years ago
Created attachment 8768635 [details] [diff] [review]
patch part 6- Suppress MJPEG fprintf() warnings in libyuv
(Assignee)

Updated

2 years ago
Attachment #8768635 - Attachment description: patch - Suppress MJPEG fprintf() warnings in libyuv → patch part 6- Suppress MJPEG fprintf() warnings in libyuv
(Assignee)

Comment 9

2 years ago
Created attachment 8768650 [details] [diff] [review]
patch part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1
(Assignee)

Comment 10

2 years ago
Created attachment 8768652 [details] [diff] [review]
patch part 8 -  Disable AVX2 asm if the compiler/assembler don't support it
(Assignee)

Updated

2 years ago
Attachment #8768650 - Attachment description: patch part 7 - Add yuv_disable_asm → patch part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1
(Assignee)

Comment 11

2 years ago
Created attachment 8768656 [details] [diff] [review]
patch part 9 - Make sure NEON ifdefs match
(Assignee)

Comment 13

2 years ago
Created attachment 8768765 [details] [diff] [review]
patch part 10 - Add toleranes to testColorConversions()
(Assignee)

Comment 14

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=80b902e05f31 

test failures of testColorConversions() seemed to be addressed.
(Assignee)

Comment 15

2 years ago
(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
(Assignee)

Comment 16

2 years ago
The build failure has triggered by "ac_add_options --enable-profiling".
(Assignee)

Comment 17

2 years ago
Created attachment 8769041 [details] [diff] [review]
patch part 11  -  Fix build failure of Android 4.2 x86 opt
(Assignee)

Comment 18

2 years ago
Created attachment 8769058 [details] [diff] [review]
patch part 12 - Update web-platform-tests webvtt ini
(Assignee)

Comment 20

2 years ago
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)
(Assignee)

Comment 21

2 years ago
Created attachment 8769524 [details] [diff] [review]
patch part 12 - Update web-platform-tests webvtt ini
Attachment #8769058 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8768349 - Flags: feedback?(rjesup) → review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768351 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768352 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768611 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768621 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768635 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768650 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
Attachment #8768652 - Flags: review?(rjesup)
(Assignee)

Updated

2 years ago
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+

Updated

2 years ago
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+

Updated

2 years ago
Attachment #8768611 - Flags: review?(rjesup) → review+

Updated

2 years ago
Attachment #8768621 - Flags: review?(rjesup) → review+

Updated

2 years ago
Attachment #8768635 - Flags: review?(rjesup) → review+

Updated

2 years ago
Attachment #8768650 - Flags: review?(rjesup) → review+

Updated

2 years ago
Attachment #8768652 - Flags: review?(rjesup) → review+

Updated

2 years ago
Attachment #8768656 - Flags: review?(rjesup) → review+
(Assignee)

Comment 25

2 years ago
(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.
(Assignee)

Updated

2 years ago
Attachment #8768765 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
Attachment #8769041 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
Attachment #8769524 - Flags: review?(james)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1282711
Attachment #8769524 - Flags: review?(james) → review+
Attachment #8768765 - Flags: review?(jmuizelaar) → review+
Attachment #8769041 - Flags: review?(jmuizelaar) → review+

Comment 27

2 years ago
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)
(Assignee)

Comment 29

2 years ago
(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 → ---
(Assignee)

Comment 33

2 years ago
(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)
(Assignee)

Comment 34

2 years ago
> 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".
(Assignee)

Updated

2 years ago
See Also: → bug 865395
(Assignee)

Updated

2 years ago
Depends on: 1288648
(Assignee)

Comment 35

2 years ago
(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.

Comment 36

2 years ago
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)
(Assignee)

Comment 39

2 years ago
(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)
(Assignee)

Comment 40

2 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #39)
> 
> Sorry, my check-in was bad :(

I forgot to add new files.

Comment 42

2 years ago
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 :)
(Assignee)

Updated

2 years ago
Blocks: 1275441
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1254872
Duplicate of this bug: 1300522
(Assignee)

Updated

a year ago
Depends on: 1308471
Depends on: 1320452
(Assignee)

Updated

a year ago
Depends on: 1329383
(Assignee)

Updated

a year ago
Depends on: 1304330

Updated

11 months ago
Blocks: 1341543
You need to log in before you can comment on or make changes to this bug.