1284803 - Update libyuv to rev 1602

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Sotaro Ikeda [:sotaro]]

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.

Comment 1

[Sotaro Ikeda [:sotaro]]

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.

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Update libyuv to rev 1602

Comment 3

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Update moz.build for fixing build failure

For addressing function definition conflict.

Comment 4

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 3 - Change basic_types.h for fixing build failure

Comment 5

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 4 - Change libyuv.gyp for fixing build failure

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 4 - Change libyuv.gyp for fixing build failure

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 5 - Enable JPEG

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 6- Suppress MJPEG fprintf() warnings in libyuv

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 7 - Disable assembly if toolchain doesn't support ssse3/sse4.1

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 8 - Disable AVX2 asm if the compiler/assembler don't support it

Comment 11

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 9 - Make sure NEON ifdefs match

Comment 12

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9bd80a151ef

Comment 13

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 10 - Add toleranes to testColorConversions()

Comment 14

[Sotaro Ikeda [:sotaro]]

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

test failures of testColorConversions() seemed to be addressed.

Comment 15

[Sotaro Ikeda [:sotaro]]

(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

Comment 16

[Sotaro Ikeda [:sotaro]]

The build failure has triggered by "ac_add_options --enable-profiling".

Comment 17

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 11 - Fix build failure of Android 4.2 x86 opt

Comment 18

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 12 - Update web-platform-tests webvtt ini

Comment 19

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=565816cd7822

Comment 20

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8768349 [details] [diff] [review]
patch part 1 - Update libyuv to rev 1602

:jesup, can you feedback to the patches?

Comment 21

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 12 - Update web-platform-tests webvtt ini

Comment 22

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed9021f7ee1f

Comment 23

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 24

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 25

[Sotaro Ikeda [:sotaro]]

(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.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
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

Comment 28

[Randell Jesup [:jesup] (needinfo me)]

Sotaro - did you try some webrtc calls (even just https://mozilla.github.io/webrtc-landing/pc_test.html)?

Comment 29

[Sotaro Ikeda [:sotaro]]

(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/.

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/92c15211f59b
https://hg.mozilla.org/mozilla-central/rev/fb3d8468b758
https://hg.mozilla.org/mozilla-central/rev/52bd1efc6c1f
https://hg.mozilla.org/mozilla-central/rev/109f79e577a0
https://hg.mozilla.org/mozilla-central/rev/6eb7bce6f4c1
https://hg.mozilla.org/mozilla-central/rev/b23773965e94
https://hg.mozilla.org/mozilla-central/rev/3b11ba39748c
https://hg.mozilla.org/mozilla-central/rev/9ea43addc3af
https://hg.mozilla.org/mozilla-central/rev/28e0ca5cb68e
https://hg.mozilla.org/mozilla-central/rev/6bacfaadbc29
https://hg.mozilla.org/mozilla-central/rev/08d2e46308fd
https://hg.mozilla.org/mozilla-central/rev/80468414501e

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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.

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backing this out for those Wr failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6428db384f6

Comment 33

[Sotaro Ikeda [:sotaro]]

(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.

Comment 34

[Sotaro Ikeda [:sotaro]]

> 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".

Comment 35

[Sotaro Ikeda [:sotaro]]

(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

[Pulsebot]

Pushed by [e-mail]:
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

Comment 37

[Iris Hsiao [:ihsiao]]

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

Comment 38

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ccccbc4cb7f
Backed out changeset 92ba454cff2a for build bustage
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b69b966591b
Backed out changeset d1cbc5eeb8c8 
https://hg.mozilla.org/integration/mozilla-inbound/rev/df3b3e7fe48e
Backed out changeset 65e1219ade06 
https://hg.mozilla.org/integration/mozilla-inbound/rev/f93076bafb61
Backed out changeset faf44eac1bb7 
https://hg.mozilla.org/integration/mozilla-inbound/rev/de6927b32ebc
Backed out changeset 24ebb285aff3 
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a195b7570b9
Backed out changeset f7f503edb55b 
https://hg.mozilla.org/integration/mozilla-inbound/rev/104e19b8f12a
Backed out changeset 6dbd0dee3572 
https://hg.mozilla.org/integration/mozilla-inbound/rev/1328c16d3bba
Backed out changeset ac2f07864fb1 
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a5831b90723
Backed out changeset 78a10fc91b52 
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3667f0527ad
Backed out changeset ccf9a31fe40d 
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1e3ccab90a9
Backed out changeset 612297895009 
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ae1c435a093
Backed out changeset 65ee637b7e20

Comment 39

[Sotaro Ikeda [:sotaro]]

(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 :(

Comment 40

[Sotaro Ikeda [:sotaro]]

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

I forgot to add new files.

Comment 41

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7c7d9977aa5

Comment 42

[Pulsebot]

Pushed by [e-mail]:
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

Comment 43

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/30ea7af8393e
https://hg.mozilla.org/mozilla-central/rev/8fc97d9901f2
https://hg.mozilla.org/mozilla-central/rev/fa3451c1f44b
https://hg.mozilla.org/mozilla-central/rev/6e7013ad8460
https://hg.mozilla.org/mozilla-central/rev/8f4c3438f844
https://hg.mozilla.org/mozilla-central/rev/df6e1dfde6e3
https://hg.mozilla.org/mozilla-central/rev/9344b69e9e32
https://hg.mozilla.org/mozilla-central/rev/e0593340378d
https://hg.mozilla.org/mozilla-central/rev/b953bf23cde2
https://hg.mozilla.org/mozilla-central/rev/3606c077fa9f
https://hg.mozilla.org/mozilla-central/rev/d4429adcef8b
https://hg.mozilla.org/mozilla-central/rev/2fc1eb070c2a

Comment 44

[Joel Maher ( :jmaher ) (UTC -8)]

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 :)