1511604 - YCbCr images display wrong colours on big-endian machines

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[A. Wilcox [:awilfox]]

libyuv is almost sort of somewhat endian-aware (but buggy about it) - however, Skia is always LE.  The libyuv code was a bit terrifying to try and dive in to so this patch just swizzles in YCbCrUtils.cpp.

Before, playing any sort of video would result in:
Crash Annotation GraphicsCriticalError: |[0][GFX1]: RGBX corner pixel at (0,0) in 512x288 surface, bounded by (0,0,512,288) is not opaque: 81,92,91,255 (t=494.788) [GFX1]: RGBX corner pixel at (0,0) in 512x288 surface, bounded by (0,0,512,288) is not opaque: 81,92,91,255
Assertion failure: [GFX1]: RGBX corner pixel at (0,0) in 512x288 surface, bounded by (0,0,512,288) is not opaque: 81,92,91,255, at /var/clean/mozppc/mozilla-unified/gfx/2d/Logging.h:728

I've tested this patch on PowerPC, and I'm able to play videos from YouTube, Vimeo, Vevo, the samples at https://www.base-n.de/webm/VP9%20Sample.html, and others.  Note that I'm only able to test this while the patch from bug 1504834 is applied; otherwise, the browser goes haywire long before media can begin playing.

Comment 1

[A. Wilcox [:awilfox]]

Attachment: Swizzle YCbCr->RGB data on big-endian machines

Tested patch on ppc64.

Comment 2

[A. Wilcox [:awilfox]]

Thank you for the review.  Is there any way to uplift this to 65 since it's tier-3 only (and therefore NPOTB)?

Comment 3

[Cameron Kaiser [:spectre]]

This needs to be checked in first. If it sticks, then the next step would be a beta request.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/571c01c5f84b
Swizzle YCbCr->RGB data on big-endian machines r=nical

Comment 5

[Noemi Erli[:noemi_erli]]

Backed out changeset 571c01c5f84b (Bug 1511604) for causing mochitest failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=571c01c5f84b7fc381ddad34b313f7ac8a62cc45&selectedJob=217187506

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=217187506&repo=mozilla-inbound&lineNumber=2785
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=217187112&repo=mozilla-inbound&lineNumber=2531
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=217186396&repo=mozilla-inbound&lineNumber=791

10:48:56  WARNING -  PROCESS-CRASH | dom/media/tests/mochitest/test_getUserMedia_mediaElementCapture_video.html | application crashed [@ mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::WriteLog(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&)]
10:48:56     INFO -  Crash dump filename: /tmp/tmpE7Oh0m/2089892d-9788-f0e1-8f71-4b369f16fe7d.dmp
10:48:56     INFO -  Operating system: Android
10:48:56     INFO -                    0.0.0 Linux 4.4.56-g594d847d09a1 #1 SMP PREEMPT Thu Oct 26 22:34:08 UTC 2017 armv8l
10:48:56     INFO -  CPU: arm
10:48:56     INFO -       ARMv1 Qualcomm part(0x51008010) features: half,thumb,fastmult,vfpv2,edsp,neon,vfpv3,tls,vfpv4,idiva,idivt
10:48:56     INFO -       8 CPUs
10:48:56     INFO -  GPU: UNKNOWN
10:48:56     INFO -  Crash reason:  SIGSEGV /SEGV_MAPERR
10:48:56     INFO -  Crash address: 0x0
10:48:56     INFO -  Process uptime: not available
10:48:56     INFO -  Thread 13 (crashed)
10:48:56     INFO -   0  libxul.so!mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::WriteLog(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) [Logging.h:571c01c5f84b7fc381ddad34b313f7ac8a62cc45 : 748 + 0x0]
10:48:56     INFO -       r0 = 0x00000000    r1 = 0xcf4eddce    r2 = 0xcf4edd72    r3 = 0x000002ec
10:48:56     INFO -       r4 = 0x000002ec    r5 = 0xcf4edd72    r6 = 0xed3b31b8    r7 = 0xd11fd230
10:48:56     INFO -       r8 = 0xd11fd29c    r9 = 0xb2101000   r10 = 0xb2101000   r12 = 0xd11fcd38
10:48:56     INFO -       fp = 0xcf353b41    sp = 0xd11fd228    lr = 0xcbc81e1d    pc = 0xcbc82314
10:48:56     INFO -      Found by: given as instruction pointer in context
10:48:56     INFO -   1  libxul.so!mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::Flush() [Logging.h:571c01c5f84b7fc381ddad34b313f7ac8a62cc45 : 286 + 0x5]
10:48:56     INFO -       r4 = 0xd11fd2a8    r5 = 0xd11fd29c    r6 = 0xed3b31b8    r7 = 0xd11fd268
10:48:56     INFO -       r8 = 0xd11fd29c    r9 = 0xb2101000   r10 = 0xb2101000    fp = 0xcf353b41
10:48:56     INFO -       sp = 0xd11fd238    lr = 0xcbc82277    pc = 0xcbc82277
10:48:56     INFO -      Found by: call frame info
10:48:56     INFO -   2  libxul.so!mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::~Log() [Logging.h:571c01c5f84b7fc381ddad34b313f7ac8a62cc45 : 279 + 0x3]
10:48:56     INFO -       r4 = 0xd11fd29c    r5 = 0xcbc81f1f    r6 = 0x00000000    r7 = 0xd11fd278
10:48:56     INFO -       r8 = 0xd11fd29c    r9 = 0xb2101000   r10 = 0xb2101000    fp = 0xcf353b41
10:48:56     INFO -       sp = 0xd11fd270    lr = 0xcbc81f43    pc = 0xcbc81f43
10:48:56     INFO -      Found by: call frame info
10:48:56     INFO -   3  libxul.so!mozilla::gfx::VerifyRGBXFormat(unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, int, mozilla::gfx::SurfaceFormat) [DrawTargetSkia.cpp:571c01c5f84b7fc381ddad34b313f7ac8a62cc45 : 162 + 0x5]
10:48:56     INFO -       r4 = 0xcbc81f05    r5 = 0xcbc81f1f    r6 = 0x00000000    r7 = 0xd11fd358
10:48:56     INFO -       r8 = 0xd11fd29c    r9 = 0xb2101000   r10 = 0xb2101000    fp = 0xcf353b41
10:48:56     INFO -       sp = 0xd11fd280    lr = 0xcbc80aa7    pc = 0xcbc80aa7
10:48:56     INFO -      Found by: call frame info

10:49:00     INFO -  12-15 02:48:23.636 F/MOZ_Assert(12322): Assertion failure: [GFX1]: RGBX pixel at (0,0) in 1280x240 surface is not opaque: 255,0,255,0, at /builds/worker/workspace/build/src/gfx/2d/Logging.h:747
10:49:00     INFO -  12-15 02:48:23.636 F/MOZ_CRASH(12322): Hit MOZ_CRASH(GFX: An assert from the graphics logger) at /builds/worker/workspace/build/src/gfx/2d/Logging.h:748
10:49:00     INFO -  12-15 02:48:23.652 W/google-breakpad(12322): ExceptionHandler::GenerateDump cloned child
10:49:00     INFO -  12-15 02:48:23.652 W/google-breakpad(12322): 13033
10:49:00     INFO -  12-15 02:48:23.652 W/google-breakpad(12322):
10:49:00     INFO -  12-15 02:48:23.652 W/google-breakpad(12322): ExceptionHandler::SendContinueSignalToChild sent continue signal to child
10:49:00     INFO -  12-15 02:48:23.656 W/google-breakpad(13033): ExceptionHandler::WaitForContinueSignal waiting for continue signal...
10:49:00     INFO -  12-15 02:48:24.052 W/InputDispatcher( 1185): channel '6dcd815 org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
10:49:00     INFO -  12-15 02:48:24.052 E/InputDispatcher( 1185): channel '6dcd815 org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp (server)' ~ Channel is unrecoverably broken and will be disposed!
10:49:00     INFO -  12-15 02:48:24.054 I/Camera2Client(  871): Camera 1: Closed
10:49:00     INFO -  12-15 02:48:24.054 I/Camera2ClientBase(  871): Closed Camera 1. Client was: org.mozilla.fennec_aurora (PID 12322, UID 10126)
10:49:00     INFO -  12-15 02:48:24.054 I/Zygote  (  733): Process 12322 exited due to signal (11)
10:49:00     INFO -  12-15 02:48:19.984 I/chatty  ( 1185): uid=1000 system_server identical 2 lines
10:49:00     INFO -  12-15 02:48:21.760 I/NotificationService( 1185): Cannot find enqueued record for key: 0|org.mozilla.fennec_aurora|2131296515|null|10126

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f44070541fb6
Backed out changeset 571c01c5f84b for causing mochitest failures CLOSED TREE

Comment 7

[Cameron Kaiser [:spectre]]

(never mind, I'm dumb)

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Looks like this got backed out because of a crash on arm

Comment 9

[msirringhaus]

I think the reason this patch failed was that it has the ifdef-guards wrong:
It has #ifdef MOZ_BIG_ENDIAN, but it should be #if MOZ_BIG_ENDIAN (MOZ_BIG_ENDIAN is 0/1 not undef/1, AFAIK).
Otherwise the swizzle occurs of LE as well, causing problems there.

The patch still applies for 68 and is needed there for BE-archs.

Would be good if it could be reconsidered.

Comment 10

[Nicolas Silva [:nical]]

Attachment: Bug 1511604 - Swizzle YCbCr->RGB data on big-endian machines. r=lsalzman

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6afcabc8f75
Swizzle YCbCr->RGB data on big-endian machines. r=lsalzman

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/f6afcabc8f75

Comment 13

[msirringhaus]

Thanks! If it works out this time around, please consider an uplift to 68esr.

Comment 14

[Mike Kaply [:mkaply]]

Comment on attachment 9093238 [details]
Bug 1511604 - Swizzle YCbCr->RGB data on big-endian machines. r=lsalzman

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed for big endian work for SUSE long term support
• User impact if declined: Things don't work on big endian
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects big endian
• String or UUID changes made by this patch:

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9093238 [details]
Bug 1511604 - Swizzle YCbCr->RGB data on big-endian machines. r=lsalzman

NPOTB fix for big-endian systems. Approved for 68.4esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/74a209297807