Fix frame hashing to include all valid planes.

Moves the frame hashing code from PipelineIntegrationTestBase to
VideoFrame per suggestions in:

https://chromiumcodereview.appspot.com/9716008/

Also disables frame hashing for BasicPlayback since it's failing on the
TSAN bots.  The code is still enabled for ffmpeg_regression_tests.

BUG=118688
TEST=media_unittests, ffmpeg_regression_tests.

Review URL: http://codereview.chromium.org/9732016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128307 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 63 insertions, 29 deletions

diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index fc97736..750c2770 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -5,6 +5,7 @@
 #include "media/base/video_frame.h"
 
 #include "base/logging.h"
+#include "base/string_piece.h"
 #include "media/base/limits.h"
 #include "media/base/video_util.h"
 
@@ -272,4 +273,16 @@ bool VideoFrame::IsEndOfStream() const {
   return format_ == VideoFrame::EMPTY;
 }
 
+void VideoFrame::HashFrameForTesting(base::MD5Context* context) {
+  for(int plane = 0; plane < kMaxPlanes; plane++) {
+    if (!IsValidPlane(plane))
+      break;
+    for(int row = 0; row < rows(plane); row++) {
+      base::MD5Update(context, base::StringPiece(
+          reinterpret_cast<char*>(data(plane) + stride(plane) * row),
+          row_bytes(plane)));
+    }
+  }
+}
+
 }  // namespace media
diff --git a/media/base/video_frame.h b/media/base/video_frame.h
index f65bf23..073fa93 100644
--- a/media/base/video_frame.h
+++ b/media/base/video_frame.h
@@ -6,6 +6,7 @@
 #define MEDIA_BASE_VIDEO_FRAME_H_
 
 #include "base/callback.h"
+#include "base/md5.h"
 #include "media/base/buffers.h"
 
 namespace media {
@@ -106,6 +107,10 @@ class MEDIA_EXPORT VideoFrame : public StreamSample {
   // StreamSample interface.
   virtual bool IsEndOfStream() const OVERRIDE;
 
+  // Used to keep a running hash of seen frames.  Expects an initialized MD5
+  // context.  Calls MD5Update with the context and the contents of the frame.
+  void HashFrameForTesting(base::MD5Context* context);
+
  private:
   // Clients must use the static CreateFrame() method to create a new frame.
   VideoFrame(Format format,
diff --git a/media/base/video_frame_unittest.cc b/media/base/video_frame_unittest.cc
index d90469b..5a6c0cd 100644
--- a/media/base/video_frame_unittest.cc
+++ b/media/base/video_frame_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -14,6 +14,8 @@
 
 namespace media {
 
+using base::MD5DigestToBase16;
+
 // Helper function that initializes a YV12 frame with white and black scan
 // lines based on the |white_to_black| parameter.  If 0, then the entire
 // frame will be black, if 1 then the entire frame will be white.
@@ -112,11 +114,21 @@ TEST(VideoFrame, CreateFrame) {
     InitializeYV12Frame(frame, 0.0f);
     ExpectFrameColor(frame, 0xFF000000);
   }
+  base::MD5Digest digest;
+  base::MD5Context context;
+  base::MD5Init(&context);
+  frame->HashFrameForTesting(&context);
+  base::MD5Final(&digest, &context);
+  EXPECT_EQ(MD5DigestToBase16(digest), "9065c841d9fca49186ef8b4ef547e79b");
   {
     SCOPED_TRACE("");
     InitializeYV12Frame(frame, 1.0f);
     ExpectFrameColor(frame, 0xFFFFFFFF);
   }
+  base::MD5Init(&context);
+  frame->HashFrameForTesting(&context);
+  base::MD5Final(&digest, &context);
+  EXPECT_EQ(MD5DigestToBase16(digest), "911991d51438ad2e1a40ed5f6fc7c796");
 
   // Test an empty frame.
   frame = VideoFrame::CreateEmptyFrame();
diff --git a/media/ffmpeg/ffmpeg_regression_tests.cc b/media/ffmpeg/ffmpeg_regression_tests.cc
index ae17dac..1fed0f3 100644
--- a/media/ffmpeg/ffmpeg_regression_tests.cc
+++ b/media/ffmpeg/ffmpeg_regression_tests.cc
@@ -18,6 +18,14 @@
 //
 // Test cases labeled FLAKY may not always pass, but they should never crash or
 // cause any kind of warnings or errors under tooling.
+//
+// Frame hashes must be generated with --video-threads=1 for correctness.
+//
+// Known issues:
+//    Cr47325 will generate an UninitValue error under Valgrind inside of the
+//    MD5 hashing code.  The error occurs due to some problematic error
+//    resilence code for H264 inside of FFmpeg.  See http://crbug.com/119020
+//
 
 #include "media/filters/pipeline_integration_test_base.h"
 
@@ -56,17 +64,17 @@ class FFmpegRegressionTest
 
 // Test cases from issues.
 FFMPEG_TEST_CASE(Cr47325, "security/47325.mp4", PIPELINE_OK, PIPELINE_OK,
-                 "bdb3976d86c531b43a4c9d43fa5e7dc2");
+                 "2a7a938c6b5979621cec998f02d9bbb6");
 FFMPEG_TEST_CASE(Cr93620, "security/93620.ogg", PIPELINE_OK, PIPELINE_OK,
                  kNullVideoHash);
 FFMPEG_TEST_CASE(Cr100492, "security/100492.webm", DECODER_ERROR_NOT_SUPPORTED,
                  DECODER_ERROR_NOT_SUPPORTED, kNullVideoHash);
 FFMPEG_TEST_CASE(Cr100543, "security/100543.webm", PIPELINE_OK, PIPELINE_OK,
-                 "17d7079458ef351b813c100ce87afbe6");
+                 "c16691cc9178db3adbf7e562cadcd6e6");
 FFMPEG_TEST_CASE(Cr101458, "security/101458.webm", DECODER_ERROR_NOT_SUPPORTED,
                  DECODER_ERROR_NOT_SUPPORTED, kNullVideoHash);
 FFMPEG_TEST_CASE(Cr108416, "security/108416.webm", PIPELINE_OK, PIPELINE_OK,
-                 "d5559b43d7406aac33bfeafceaf86fe9");
+                 "5cb3a934795cd552753dec7687928291");
 FFMPEG_TEST_CASE(Cr110849, "security/110849.mkv", DEMUXER_ERROR_COULD_NOT_PARSE,
                  DEMUXER_ERROR_COULD_NOT_PARSE, kNullVideoHash);
 FFMPEG_TEST_CASE(Cr112384, "security/112384.webm",
@@ -149,7 +157,7 @@ FFMPEG_TEST_CASE(OGV_18, "security/wav.711.ogv", DECODER_ERROR_NOT_SUPPORTED,
 
 // General WebM test cases.
 FFMPEG_TEST_CASE(WEBM_1, "security/no-bug.webm", PIPELINE_OK, PIPELINE_OK,
-                 "20536f9ce571c5830bb7c782584223fd");
+                 "39e92700cbb77478fd63f49db855e7e5");
 FFMPEG_TEST_CASE(WEBM_2, "security/uninitialize.webm", PIPELINE_ERROR_DECODE,
                  PIPELINE_ERROR_DECODE, kNullVideoHash);
 FFMPEG_TEST_CASE(WEBM_3, "security/out.webm.139771.2965",
@@ -159,34 +167,34 @@ FFMPEG_TEST_CASE(WEBM_4, "security/out.webm.68798.1929",
                  DECODER_ERROR_NOT_SUPPORTED, DECODER_ERROR_NOT_SUPPORTED,
                  kNullVideoHash);
 FFMPEG_TEST_CASE(WEBM_5, "content/frame_size_change.webm", PIPELINE_OK,
-                 PIPELINE_OK, "0e89c98da281067d58f8459eca0f35ba");
+                 PIPELINE_OK, "d8fcf2896b7400a2261bac9e9ea930f8");
 FFMPEG_TEST_CASE(WEBM_6, "security/117912.webm", DEMUXER_ERROR_COULD_NOT_OPEN,
                  DEMUXER_ERROR_COULD_NOT_OPEN, kNullVideoHash);
 
 // Flaky under threading, maybe larger issues.  Values were set with
 // --video-threads=1 under the hope that they may one day pass with threading.
 FFMPEG_TEST_CASE(FLAKY_Cr99652, "security/99652.webm", PIPELINE_OK,
-                 PIPELINE_ERROR_DECODE, "43db203056009b7ff3ac145a3488807a");
+                 PIPELINE_ERROR_DECODE, "97956dca8897484fbd0dd1169578adbf");
 FFMPEG_TEST_CASE(FLAKY_Cr100464, "security/100464.webm", PIPELINE_OK,
-                 PIPELINE_OK, "985cff66e292f3064bf853e8943fd8c8");
+                 PIPELINE_OK, "a32ddb4ac5ca89429e1a3c968e876ce8");
 FFMPEG_TEST_CASE(FLAKY_OGV_3, "security/smclock_1_0.ogv", PIPELINE_OK,
-                 PIPELINE_OK, "6923c1e1c6354d8e00cd57b7b66c9b02");
+                 PIPELINE_OK, "2f7aa71ce789e47a1dde98ecdd774679");
 FFMPEG_TEST_CASE(FLAKY_OGV_4, "security/smclock.ogv.1.0.ogv",
-                 PIPELINE_OK, PIPELINE_OK, "6923c1e1c6354d8e00cd57b7b66c9b02");
+                 PIPELINE_OK, PIPELINE_OK, "2f7aa71ce789e47a1dde98ecdd774679");
 FFMPEG_TEST_CASE(FLAKY_OGV_13, "security/smclocktheora_1_790.ogv",
-                 PIPELINE_OK, PIPELINE_OK, "6923c1e1c6354d8e00cd57b7b66c9b02");
+                 PIPELINE_OK, PIPELINE_OK, "2f7aa71ce789e47a1dde98ecdd774679");
 FFMPEG_TEST_CASE(FLAKY_MP4_4, "security/clockh264aac_301350139.mp4",
-                 PIPELINE_OK, PIPELINE_OK, "87c110e9d0bbaf7a2d9757dc8e2ce9d7");
+                 PIPELINE_OK, PIPELINE_OK, "bf1dbeb4ee6edb1b6c78742f4943c162");
 
-// Flaky due to other non-threading related reasons:
+// Flaky due to other non-threading related reasons.
 FFMPEG_TEST_CASE(FLAKY_Cr111342, "security/111342.ogm", PIPELINE_OK,
-                 PIPELINE_ERROR_DECODE, "a79faa6f4ccb1734e2143d5d37482448");
+                 PIPELINE_ERROR_DECODE, "8a14eb9ce0671194a0b04fee8b0b5368");
 FFMPEG_TEST_CASE(FLAKY_OGV_0, "security/big_dims.ogv", PIPELINE_OK,
-                 PIPELINE_OK, "c5c23c1e0958a4a4756c35d045f94e0e");
+                 PIPELINE_OK, "c76ac9c5e9f3b8edaa341d9ee74739f4");
 FFMPEG_TEST_CASE(FLAKY_OGV_6, "security/smclocktheora_1_10000.ogv",
-                 PIPELINE_OK, PIPELINE_OK, "cdce61e17a8058c6ac5fca92cd3e4a86");
+                 PIPELINE_OK, PIPELINE_OK, "b16734719247275c9d9c828d25ffd4bf");
 FFMPEG_TEST_CASE(FLAKY_MP4_3, "security/clockh264aac_300413969.mp4",
-                 PIPELINE_OK, PIPELINE_OK, "3781a90401195dfb6ad3e4dbc4888f4f");
+                 PIPELINE_OK, PIPELINE_OK, "b7659b127be88829a68d8a9aa625795e");
 
 // Hangs. http://crbug.com/117038
 // FFMPEG_TEST_CASE(WEBM_0, "security/memcpy.webm", PIPELINE_OK, PIPELINE_OK);
diff --git a/media/filters/pipeline_integration_test.cc b/media/filters/pipeline_integration_test.cc
index d8d5f72..c5d98fb 100644
--- a/media/filters/pipeline_integration_test.cc
+++ b/media/filters/pipeline_integration_test.cc
@@ -127,7 +127,10 @@ TEST_F(PipelineIntegrationTest, BasicPlayback) {
   Play();
 
   ASSERT_TRUE(WaitUntilOnEnded());
-  ASSERT_EQ(GetVideoHash(), "923ce08df753797b642df55a46a1dcfb");
+
+  // TODO(dalecurtis): Due to threading issues in FFmpeg, frames are not always
+  // decoded exactly, see http://crbug.com/93932 and http://crbug.com/109875.
+  // ASSERT_EQ(GetVideoHash(), "f0be120a90a811506777c99a2cdf7cc1");
 }
 
 // TODO(xhwang): Enable this test when AddKey is integrated into pipeline.
diff --git a/media/filters/pipeline_integration_test_base.cc b/media/filters/pipeline_integration_test_base.cc
index e421963..91ca357 100644
--- a/media/filters/pipeline_integration_test_base.cc
+++ b/media/filters/pipeline_integration_test_base.cc
@@ -5,7 +5,6 @@
 #include "media/filters/pipeline_integration_test_base.h"
 
 #include "base/bind.h"
-#include "base/string_piece.h"
 #include "media/base/media_log.h"
 #include "media/filters/chunk_demuxer_factory.h"
 #include "media/filters/ffmpeg_audio_decoder.h"
@@ -18,6 +17,8 @@ using ::testing::AnyNumber;
 
 namespace media {
 
+const char kNullVideoHash[] = "d41d8cd98f00b204e9800998ecf8427e";
+
 PipelineIntegrationTestBase::PipelineIntegrationTestBase()
     : message_loop_factory_(new MessageLoopFactory()),
       pipeline_(new Pipeline(&message_loop_, new MediaLog())),
@@ -185,12 +186,7 @@ void PipelineIntegrationTestBase::OnVideoRendererPaint() {
   scoped_refptr<VideoFrame> frame;
   renderer_->GetCurrentFrame(&frame);
   if (frame)
-    base::MD5Update(
-        &md5_context_,
-        base::StringPiece(
-            reinterpret_cast<char*>(frame->data(VideoFrame::kRGBPlane)),
-            (frame->rows(VideoFrame::kRGBPlane) *
-             frame->row_bytes(VideoFrame::kRGBPlane))));
+    frame->HashFrameForTesting(&md5_context_);
   renderer_->PutCurrentFrame(frame);
 }
 
diff --git a/media/filters/pipeline_integration_test_base.h b/media/filters/pipeline_integration_test_base.h
index dfa231b..4fb65d0 100644
--- a/media/filters/pipeline_integration_test_base.h
+++ b/media/filters/pipeline_integration_test_base.h
@@ -17,7 +17,7 @@
 namespace media {
 
 // Empty MD5 hash string.  Used to verify videos which have decoded no frames.
-static const char kNullVideoHash[] = "d41d8cd98f00b204e9800998ecf8427e";
+extern const char kNullVideoHash[];
 
 // Integration tests for Pipeline. Real demuxers, real decoders, and
 // base renderer implementations are used to verify pipeline functionality. The
diff --git a/tools/valgrind/gtest_exclude/media_unittests.gtest-tsan_win32.txt b/tools/valgrind/gtest_exclude/media_unittests.gtest-tsan_win32.txt
index e8b2dd6..9185520 100644
--- a/tools/valgrind/gtest_exclude/media_unittests.gtest-tsan_win32.txt
+++ b/tools/valgrind/gtest_exclude/media_unittests.gtest-tsan_win32.txt
@@ -1,6 +1,3 @@
-# Fails, probably due to the suppressed data races. See http://crbug.com/93932
-PipelineIntegrationTest.BasicPlayback
-
 # This test fails reliably in tsan bots after r119048, exclude it for
 # now. See  http://crbug.com/109875
 PipelineIntegrationTest.SeekWhilePlaying