561412 - new yuv -> rgb conversion code assumes GTK=linux

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Hanspeter Niederstrasser]

Related to bug 551277:

On Darwin/gtk2, the build fails in gfx/ycbcr 

g++-4.2 -arch i386 -o yuv_row_linux.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  -DZLIB_INTERNAL -DOSTYPE=\"Darwin\" -DOSARCH=Darwin  -I/src/mozilla-central/gfx/ycbcr -I. -I../../dist/include -I../../dist/include/nsprpub  -I/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/dist/include/nspr -I/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/dist/include/nss -I/sw/include      -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 -Wno-long-long -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -fshort-wchar -pthread  -DNDEBUG -DTRIMMED -O3   -DMOZILLA_CLIENT -include ../../mozilla-config.h -Wp,-MD,.deps/yuv_row_linux.pp /src/mozilla-central/gfx/ycbcr/yuv_row_linux.cpp
/var/folders/VJ/VJI1GqIRGmOqqRYrL1IClU+++TI/-Tmp-//cc3LonnM.s:1:Unknown pseudo-op: .global
/var/folders/VJ/VJI1GqIRGmOqqRYrL1IClU+++TI/-Tmp-//cc3LonnM.s:1:Rest of line ignored. 1st junk character valued 70 (F).
make[5]: *** [yuv_row_linux.o] Error 1
make[5]: Leaving directory `/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/gfx/ycbcr'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/gfx'

Makefile.in uses MOZ_WIDGET_TOOLKIT to determine which yuv_row_XXX.cpp file to build (windows, gtk2, cocoa, other).  Switching to OS_ARCH as the determinant makes Darwin compile yuv_row_mac.cpp instead of yuv_row_linux.cpp, which lets the build finish correctly (and the test video from bug 551277 comment 32 plays correctly).

Comment 1

[Hanspeter Niederstrasser]

Attachment: changes MOZ_WIDGET_TOOLKIT to OS_ARCH

Patch changes gfx/ycbcr/Makefile.in to use OS_ARCH instead of MOZ_WIDGET_TOOLKIT.

Comment 2

[Jacek Caban]

As I've written in bug 561344, Windows version should be based on compiler (_MSC_VER), not OS.

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 441088 [details] [diff] [review]
changes MOZ_WIDGET_TOOLKIT to OS_ARCH

You probably want to request review from roc on this. [/me adds review request]

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 441088 [details] [diff] [review]
changes MOZ_WIDGET_TOOLKIT to OS_ARCH

see comment #2

Comment 5

[Hanspeter Niederstrasser]

Attachment: uses OS_ARCH and _MSC_VER

Jacek, sorry, I completely missed bug 561344.  Updated the patch here to use _MSC_VER for windows, but OS_ARCH for Linux and Mac.

Comment 6

[Hanspeter Niederstrasser]

Attachment: OS_ARCH only for Linux and Mac

Bug 561344 got fixed, so I removed the Windows part of the patch (keep the code with the new _MSC_VER check)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 442656 [details] [diff] [review]
OS_ARCH only for Linux and Mac

I'm not sure why all gcc builds can't use the same source, but this is definitely an improvement.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/70e1091d44b9