Lift MEDIA_MMX_INTRINSICS_AVAILABLE definition to header and use it to condition media_unittests

media/base/yuv_convert_unittest.cc

Index: media/base/yuv_convert_unittest.cc
diff --git a/media/base/yuv_convert_unittest.cc b/media/base/yuv_convert_unittest.cc
index c57f71521534c363cdee231c004d84bd1281a15b..f51e6d5d1f4c56fe2b74c7183e0c25e8cd87bde7 100644
--- a/media/base/yuv_convert_unittest.cc
+++ b/media/base/yuv_convert_unittest.cc
@@ -776,6 +776,7 @@ TEST(YUVConvertTest, FilterYUVRows_C_OutOfBounds) {
 }
 
 TEST(YUVConvertTest, FilterYUVRows_MMX_OutOfBounds) {

[scherkus (not reviewing), 2013/02/06 08:09:01]

gtest supports disabling tests by prepending the test name with DISABLED_

this way the code is still compiled + linked, but is skipped by default

can you change to the following?

#if defined(MEDIA_MMX_INTRINSICS_AVAILABLE)
TEST(YUVConvertTest, DISABLED_FilterYUVRows_MMX_OutOfBounds) {
#else
TEST(YUVConvertTest, FilterYUVRows_MMX_OutOfBounds) {
#endif

[wolenetz, 2013/02/06 19:03:10]

Sorry about being pedantic here.  Please help me understand your comment:
If I understand correctly, the logic would be reverse of your proposal: if MMX
intrinsics are available, then go ahead and test the method if the cpu also
supports mmx, otherwise disable the test.
On win64, the disabled, but still compiled and linked test, would not enable
successful build because the routine that uses MMX intrinsics wouldn't be built
into media.dll, so linking would fail. On win64, the routine is excluded from
media.dll because it uses MMX intrinsics that VS2010 doesn't support
(compilation of media.dll would fail if included).

If the scope of the win64-disabled version of the test were changed to be
essentially a noop (or the current LOG(WARNING)), then using DISABLED_ would
work, I think.

+#if defined(MEDIA_MMX_INTRINSICS_AVAILABLE)
   base::CPU cpu;
   if (!cpu.has_mmx()) {
     LOG(WARNING) << "System not supported. Test skipped.";
@@ -795,6 +796,10 @@ TEST(YUVConvertTest, FilterYUVRows_MMX_OutOfBounds) {
   for (int i = 1; i < 16; ++i) {
     EXPECT_EQ(0u, dst[i]);
   }
+#else
+  LOG(WARNING) << "System not supported. Test skipped.";
+  return;
+#endif  // defined(MEDIA_MMX_INTRINSICS_AVAILABLE)
 }
 
 TEST(YUVConvertTest, FilterYUVRows_SSE2_OutOfBounds) {
@@ -819,6 +824,7 @@ TEST(YUVConvertTest, FilterYUVRows_SSE2_OutOfBounds) {
 }
 
 TEST(YUVConvertTest, FilterYUVRows_MMX_UnalignedDestination) {
+#if defined(MEDIA_MMX_INTRINSICS_AVAILABLE)

[scherkus (not reviewing), 2013/02/06 08:09:01]

ditto

[wolenetz, 2013/02/06 19:03:10]

See reply for similar comment, above.

   base::CPU cpu;
   if (!cpu.has_mmx()) {
     LOG(WARNING) << "System not supported. Test skipped.";
@@ -846,6 +852,10 @@ TEST(YUVConvertTest, FilterYUVRows_MMX_UnalignedDestination) {
   media::EmptyRegisterState();
 
   EXPECT_EQ(0, memcmp(dst_sample.get(), dst_ptr, 17));
+#else
+  LOG(WARNING) << "System not supported. Test skipped.";
+  return;
+#endif  // defined(MEDIA_MMX_INTRINSICS_AVAILABLE)
 }
 
 TEST(YUVConvertTest, FilterYUVRows_SSE2_UnalignedDestination) {