Closed
Bug 620526
Opened 14 years ago
Closed 14 years ago
yv12_to_rgb565_neon should not be used on CPUs without NEON
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0b4+ | --- |
People
(Reporter: mwu, Assigned: romaxa)
References
Details
(Keywords: crash)
Attachments
(3 files)
(deleted),
patch
|
jrmuizel
:
review+
blassey
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/x-c++src
|
Details |
Not all modern ARM cpus have NEON. In particular, the tegra does not have it.
Updated•14 years ago
|
tracking-fennec: ? → 2.0b4+
Assignee | ||
Comment 1•14 years ago
|
||
Comment 2•14 years ago
|
||
Comment on attachment 498966 [details] [diff] [review] ARM neon check this certainly looks right to me, but its not my module
Attachment #498966 -
Flags: review?(chris.double)
Attachment #498966 -
Flags: review?(blassey.bugs)
Attachment #498966 -
Flags: feedback+
Reporter | ||
Comment 3•14 years ago
|
||
(In reply to comment #1) > Created attachment 498966 [details] [diff] [review] > ARM neon check I meant we need to have a runtime check.
Comment 4•14 years ago
|
||
(In reply to comment #3) > (In reply to comment #1) > > Created attachment 498966 [details] [diff] [review] [details] > > ARM neon check > > I meant we need to have a runtime check. both would be good, but yea you'd need a runtime check for the tegra to work
Assignee | ||
Comment 5•14 years ago
|
||
something like this: http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#223 ?
Comment 6•14 years ago
|
||
yea. We currently do just about the same check in jstracer.cpp and jstracer.c.
Reporter | ||
Comment 7•14 years ago
|
||
(In reply to comment #5) > something like this: > http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#223 > ? Well, closer to http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#247 but yeah, need some variant of it. There's also cpu detection in media/libvpx/vpx_ports/arm_cpudetect.c and another cpu detector in js. We have too many of them.
Comment 8•14 years ago
|
||
> Well, closer to
> http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#247
How about something like this?
static pixman_bool_t
pixman_have_ycpcr_to_rgb565 ()
{
int fd;
Elf32_auxv_t aux;
pixman_bool_t arm_has_neon = 0;
fd = open ("/proc/self/auxv", O_RDONLY);
if (fd >= 0)
{
while (read (fd, &aux, sizeof(Elf32_auxv_t)) == sizeof(Elf32_auxv_t))
{
if (aux.a_type == AT_HWCAP)
{
arm_has_neon = (hwcap & 4096) != 0;
break;
}
}
close (fd);
}
return arm_has_neon;
}
Assignee | ||
Comment 9•14 years ago
|
||
tom could you attach working patch and ask for review?
Comment 10•14 years ago
|
||
I copied the logic from piximan-cpu file and moved it here for runtime test in ycbcr folder. This file could easily be moved to a more central common location. There is a test case in the following attachment.
Attachment #501747 -
Flags: review?(mwu)
Comment 11•14 years ago
|
||
Reporter | ||
Comment 12•14 years ago
|
||
Comment on attachment 501747 [details] [diff] [review] Patch - have_ycbcr_to_rgb565 I suspect Chris Double is the right reviewer for this.
Attachment #501747 -
Flags: review?(mwu) → review?(chris.double)
Updated•14 years ago
|
Attachment #498966 -
Flags: review?(chris.double) → review?(jmuizelaar)
Updated•14 years ago
|
Attachment #501747 -
Flags: review?(chris.double) → review?(jmuizelaar)
Updated•14 years ago
|
Attachment #498966 -
Flags: review?(jmuizelaar) → review+
Updated•14 years ago
|
Attachment #501747 -
Flags: review?(jmuizelaar) → review+
Comment 13•14 years ago
|
||
g++-4.2 -arch i386 -o ycbcr_to_rgb565.o -c -fvisibility=hidden -D_IMPL_NS_GFX -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES -DSTATIC_EXPORTABLE_JS_API -DOSTYPE=\"Darwin\" -DOSARCH=Darwin -I/builds/slave/try-osx64/build/gfx/ycbcr -I. -I../../dist/include -I../../dist/include/nsprpub -I/builds/slave/try-osx64/build/obj-firefox/i386/dist/include/nspr -I/builds/slave/try-osx64/build/obj-firefox/i386/dist/include/nss -fPIC -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -fshort-wchar -pthread -DNO_X11 -DNDEBUG -DTRIMMED -gdwarf-2 -O3 -fomit-frame-pointer -DMOZILLA_CLIENT -include ../../mozilla-config.h /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:60:17: error: elf.h: No such file or directory /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp: In function 'int have_ycbcr_to_rgb565()': /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:96: error: 'Elf32_auxv_t' was not declared in this scope /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:96: error: expected `;' before 'aux' /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:101: error: 'aux' was not declared in this scope /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:103: error: 'AT_HWCAP' was not declared in this scope /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:105: error: 'uint32_t' was not declared in this scope /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:105: error: expected `;' before 'hwcap' /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:106: error: 'hwcap' was not declared in this scope Does not compile on the mac.
Comment 14•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/c0db6f2597a2 with build bustage fixes.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 15•14 years ago
|
||
Comment on attachment 501747 [details] [diff] [review] Patch - have_ycbcr_to_rgb565 So, for those of you still building non-libxul: (3x) >+int have_ycbcr_to_rgb565 () NS_GFX_(int) have_ycbcr_to_rgb565 () >+int have_ycbcr_to_rgb565(); #include "gfxCore.h" NS_GFX_(int) have_ycbcr_to_rgb565();
You need to log in
before you can comment on or make changes to this bug.
Description
•