Add vaapi_h264_decoder_test.

content/common/gpu/media/vaapi_h264_decoder_unittest.cc

Index: content/common/gpu/media/vaapi_h264_decoder_unittest.cc
diff --git a/content/common/gpu/media/vaapi_h264_decoder_unittest.cc b/content/common/gpu/media/vaapi_h264_decoder_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..209cc889276b6cf55f2f245205761d56264ce697
--- /dev/null
+++ b/content/common/gpu/media/vaapi_h264_decoder_unittest.cc
@@ -0,0 +1,386 @@
+// Copyright 2013 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.
+
+#include <string>
+
+// This has to be included first.
+// See http://code.google.com/p/googletest/issues/detail?id=371
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "content/common/gpu/media/vaapi_h264_decoder.h"
+#include "media/base/video_decoder_config.h"
+
+// This program is run like this:
+// DISPLAY=:0 ./vaapi_h264_decoder_unittest --input_file input.264

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

s/264/h264/ to line up with kDefaultInputFile

[chihchung, 2013/12/09 05:10:08]

Done.

+// [--output_file output.yuv] [--md5sum expected_md5_hex]

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

Can you s/yuv/i420/g throughout this file, for precision?

[chihchung, 2013/12/09 05:10:08]

Done.

+//
+// The input is read from input.264. The output is written to output.yuv if it
+// is given. It also verifies the MD5 sum of the decoded YUV data if the
+// expected MD5 sum is given.
+
+namespace content {
+namespace {
+
+// These are the command line parameters
+base::FilePath g_input_file;
+base::FilePath g_output_file;
+std::string g_md5sum;
+
+// These default values are used if nothing is specified in the command line.
+const base::FilePath::CharType* kDefaultInputFile =
+    FILE_PATH_LITERAL("test-25fps.h264");
+const char* kDefaultMD5Sum = "3af866863225b956001252ebeccdb71d";
+
+// This class encapsulates the use of VaapiH264Decoder to a simpler interface.
+// It reads an H.264 Annex B bytestream from a file and outputs the decoded
+// frames (in I420 format) to another file.

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

If you emitted http://wiki.multimedia.cx/index.php?title=YUV4MPEG2 then various
tools could play back the results more easily (and the container should be
trivial to hand-craft).  Up to you.

[chihchung, 2013/12/09 05:10:08]

I use this command line to play it:
mplayer test_dec.yuv -demuxer rawvideo -rawvideo w=1920:h=1080

The raw i420 format is to match the golden result in the conformance test.

+//
+// To use the class, construct an instance, call Initialize() to specify the
+// input and output file path, then call Run() to decode the whole stream and

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

s/path/paths/

[chihchung, 2013/12/09 05:10:08]

Done.

+// output the frames.
+//
+// This class must be created, called and destroyed on a single thread, and
+// does nothing internally on any other thread.
+class VaapiH264DecoderLoop {
+ public:
+  VaapiH264DecoderLoop();
+  ~VaapiH264DecoderLoop();
+
+  // Initialize the decoder. Return true if successful.
+  bool Initialize(base::FilePath input_file, base::FilePath output_file);
+
+  // Run the decode loop. The decoded data is written to the file specified by
+  // output_file in Initialize().  Return true if all decoding is successful.
+  bool Run();
+
+  // Get the MD5 sum of the decoded data.
+  std::string GetMD5Sum();
+
+ private:
+  void ReportToUMA(VaapiH264Decoder::VAVDAH264DecoderFailure error) {}

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

s/ReportToUMA/BlackholeErrorCodes/
s/error//

although TBH I'd prefer
void CrashOnError(VaapiH264Decoder::VAVDAH264DecoderFailure error) {
  LOG(FATAL) << "Oh noes!  Decoder failed: " << error;
}

There's also no need for this to be a member function (could just as well be a
free file-static function), which would allow you to drop a base::Unretained()
below.   Unretained is generally a code-smell unless you have rationale about
why it is safe.

[chihchung, 2013/12/09 05:10:08]

Done.

+
+  // Callback from the decoder when a picture is decoded.
+  void OutputPicture(int32 input_id,
+                     const scoped_refptr<VASurface>& va_surface);
+
+  // Recycle one surface and put it on available_surfaces_ list.
+  void RecycleSurface(VASurfaceID va_surface_id);
+
+  // Give all surfaces in available_surfaces_ to the decoder.
+  void RefillSurfaces();
+
+  // Free the current set of surfaces and allocate a new set of
+  // surfaces. Returns true when successful.
+  bool AllocateNewSurfaces();
+
+  // Use the data in the frame: write to file and update MD5 sum.
+  bool ProcessVideoFrame(const scoped_refptr<media::VideoFrame>& frame);
+
+  scoped_ptr<VaapiH264Decoder> decoder_;
+  scoped_ptr<VaapiWrapper> wrapper_;
+  std::string data_;            // data read from input_file
+  base::FilePath output_file_;  // output data is written to this file
+  std::vector<VASurfaceID> available_surfaces_;
+
+  // These members (x_display_, num_outputted_pictures_, num_surfaces_)
+  // need to be initialized and possibly freed manually.
+  Display* x_display_;
+  int num_outputted_pictures_;  // number of pictures already outputted
+  size_t num_surfaces_;  // number of surfaces in the current set of surfaces
+  base::MD5Context md5_context_;
+};
+
+VaapiH264DecoderLoop::VaapiH264DecoderLoop()
+    : x_display_(NULL), num_outputted_pictures_(0), num_surfaces_(0) {
+  base::MD5Init(&md5_context_);
+}
+
+VaapiH264DecoderLoop::~VaapiH264DecoderLoop() {
+  // We need to destruct decoder and wrapper first because:

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

Then wrapper_ should be declared before decoder_ in the class layout (C++
destruction works from the bottom up, so manual tear-down should work similarly
for minimum confusion).

[chihchung, 2013/12/09 05:10:08]

Done.

+  // (1) The decoder has a reference to the wrapper.
+  // (2) The wrapper has a reference to x_display_.
+  decoder_.reset();
+  wrapper_.reset();
+
+  if (x_display_) {
+    XCloseDisplay(x_display_);
+  }
+}
+
+bool VaapiH264DecoderLoop::Initialize(base::FilePath input_file,
+                                      base::FilePath output_file) {
+  x_display_ = XOpenDisplay(NULL);
+  if (!x_display_) {
+    LOG(ERROR) << "Can't open X display";
+    return false;

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

In general for programs like these I prefer LOG(FATAL) to LOG(ERROR)+return
false because it simplifies code everywhere (both caller & impl).  E.g.
l.117-121 can just be:
CHECK(x_display_ = XOpenDisplay(NULL));

If you take this suggestions then the contents of Initialize() can migrate into
the ctor.

[chihchung, 2013/12/09 05:10:08]

I think there might be a chance for this class to be used in a larger program,
so I think we can leave it as is now.

[Ami GONE FROM CHROMIUM, 2013/12/09 18:55:56]

I hope not! :)
(libva as an API is pretty terrible; I hope we don't end up with more
dependencies on it at this level, rather than the VDA level)

+  }
+
+  media::VideoCodecProfile profile = media::H264PROFILE_HIGH;

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

Doesn't this need to be a cmd-line param in case different inputs use different
profiles?

[chihchung, 2013/12/09 05:10:08]

I checked the libva-intel-driver code, and it doesn't distinguish profiles in
H.264 at all (it only distinguish things like H.264 vs MPEG2).

[Ami GONE FROM CHROMIUM, 2013/12/09 18:55:56]

I think you mean you checked the implementation, but this file doesn't use the
libva implementation; it uses its interface.  It would be nice to avoid
hard-coding today's libva's impl details into this test for the future.

E.g. of where this could bite you: libva impl is changed to allocate only
resources necessary for the specified profile.  A baseline profile video
incorrectly requires too many resources.  This test wouldn't catch that b/c it
was still asking for HIGH when it should be asking for BASELINE.

[chihchung, 2013/12/24 07:33:47]

I changed it to baseline, so if the implementation changes, we can easily catch
that.

On 2013/12/09 18:55:56, Ami Fischman (OOO Dec23-Jan5) wrote:

+  base::Closure report_error_cb = base::Bind(&VaapiH264DecoderLoop::ReportToUMA,
+                                             base::Unretained(this),
+                                             VaapiH264Decoder::VAAPI_ERROR);
+  wrapper_ = VaapiWrapper::Create(profile, x_display_, report_error_cb);
+  if (!wrapper_.get()) {
+    LOG(ERROR) << "Can't create vaapi wrapper";
+    return false;
+  }
+
+  decoder_.reset(new VaapiH264Decoder(
+      wrapper_.get(),
+      base::Bind(&VaapiH264DecoderLoop::OutputPicture, base::Unretained(this)),
+      base::Bind(&VaapiH264DecoderLoop::ReportToUMA, base::Unretained(this))));
+
+  if (!base::ReadFileToString(input_file, &data_)) {
+    LOG(ERROR) << "failed to read input data from " << input_file.value();
+    return false;
+  }
+
+  const int input_id = 0;  // We don't use input_id in this class.
+  decoder_->SetStream(
+      reinterpret_cast<const uint8*>(data_.c_str()), data_.size(), input_id);
+
+  // This creates or truncates output_file.
+  // Without it, AppendToFile() will not work.
+  if (!output_file.empty()) {
+    if (file_util::WriteFile(output_file, NULL, 0) != 0) {
+      return false;
+    }
+    output_file_ = output_file;
+  }
+
+  return true;
+}
+
+bool VaapiH264DecoderLoop::Run() {
+  while (1) {
+    switch (decoder_->Decode()) {
+      case VaapiH264Decoder::kDecodeError:
+        LOG(ERROR) << "Decode Error";
+        return false;
+      case VaapiH264Decoder::kAllocateNewSurfaces:
+        VLOG(1) << "Allocate new surfaces";
+        if (!AllocateNewSurfaces()) {
+          LOG(ERROR) << "Failed to allocate new surfaces";
+          return false;
+        }
+        break;
+      case VaapiH264Decoder::kRanOutOfStreamData: {
+        bool rc = decoder_->Flush();
+        VLOG(1) << "Flush returns " << rc;
+        return rc;
+      }
+      case VaapiH264Decoder::kRanOutOfSurfaces:
+        VLOG(1) << "Ran out of surfaces";
+        RefillSurfaces();
+        break;
+    }
+  }
+}
+
+std::string VaapiH264DecoderLoop::GetMD5Sum() {

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

static

[chihchung, 2013/12/09 05:10:08]

It uses a md5_context_ member variable.

+  base::MD5Digest digest;
+  base::MD5Final(&digest, &md5_context_);
+  return MD5DigestToBase16(digest);
+}
+
+scoped_refptr<media::VideoFrame> CopyNV12ToI420(VAImage* image, void* mem) {

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

libyuv::NV12ToI420 ?

+  DVLOG(1) << "CopyNV12ToI420 width=" << image->width
+           << ", height=" << image->height;
+
+  const gfx::Size coded_size(image->width, image->height);
+  const gfx::Rect visible_rect(image->width, image->height);
+  const gfx::Size natural_size(image->width, image->height);
+
+  scoped_refptr<media::VideoFrame> frame =
+      media::VideoFrame::CreateFrame(media::VideoFrame::I420,
+                                     coded_size,
+                                     visible_rect,
+                                     natural_size,
+                                     base::TimeDelta());
+
+  uint8_t* mem_byte_ptr = static_cast<uint8_t*>(mem);
+  uint8_t* srcY = mem_byte_ptr + image->offsets[0];
+  uint8_t* srcU = mem_byte_ptr + image->offsets[1];
+  uint8_t* srcV = srcU + 1;
+
+  uint8_t* dstY = frame->data(media::VideoFrame::kYPlane);
+  uint8_t* dstU = frame->data(media::VideoFrame::kUPlane);
+  uint8_t* dstV = frame->data(media::VideoFrame::kVPlane);
+
+  for (int i = 0; i < image->height; i++) {
+    memcpy(dstY, srcY, image->width);
+    srcY += image->pitches[0];
+    dstY += frame->stride(media::VideoFrame::kYPlane);
+
+    // U/V components are subsampled, so there is only one U/V row for every two
+    // Y rows.
+    if ((i & 1) == 1)
+      continue;
+    for (int j = 0, k = 0; j < image->width; j += 2, k++) {
+      dstU[k] = srcU[j];
+      dstV[k] = srcV[j];
+    }
+    srcU += image->pitches[1];
+    srcV += image->pitches[1];
+    dstU += frame->stride(media::VideoFrame::kUPlane);
+    dstV += frame->stride(media::VideoFrame::kVPlane);
+  }
+
+  return frame;
+}
+
+bool VaapiH264DecoderLoop::ProcessVideoFrame(
+    const scoped_refptr<media::VideoFrame>& frame) {
+  for (size_t i = 0; i < media::VideoFrame::NumPlanes(frame->format()); i++) {
+    uint8* data = frame->data(i);
+    int bytes = frame->row_bytes(i);
+    for (int j = 0; j < frame->rows(i); j++) {
+      const char* buf = reinterpret_cast<const char*>(data);
+      base::MD5Update(&md5_context_, base::StringPiece(buf, bytes));

[Ami GONE FROM CHROMIUM, 2013/12/05 02:11:43]

The MD5 part of this function can be satisfied instead by
VideoFrame::HashFrameForTesting().

The rest of this function appears to be written to deal with non-zero padding /
alignment / stride concerns, but l.195-197 say that's not something you're
worried about, so I think you can collapse this function to simply:
frame->HashFrameForTesting(&md5_context_);
int to_write = frame->AllocationSize(frame->format(), frame->coded_size();
int written = file_util::AppendToFile(output_file_, frame->data(kYPlane),
to_write);
CHECK_EQ(written, to_write);
?

[chihchung, 2013/12/09 05:10:08]

Done. Thanks a lot for the suggestion. I use one write call for each plane,
because I am not sure there is no paddings between the three planes.

+      if (!output_file_.empty()) {
+        int written = file_util::AppendToFile(output_file_, buf, bytes);
+        if (written != bytes) {
+          return false;
+        }
+      }
+      data += frame->stride(i);
+    }
+  }
+  return true;
+}
+
+void VaapiH264DecoderLoop::OutputPicture(
+    int32 input_id,
+    const scoped_refptr<VASurface>& va_surface) {
+  VLOG(1) << "OutputPicture: picture " << num_outputted_pictures_++;
+
+  VAImage image;
+  void* mem;
+
+  if (!wrapper_->GetVaImageForTesting(va_surface->id(), &image, &mem)) {
+    LOG(ERROR) << "Cannot get VAImage.";
+    return;
+  }
+
+  if (image.format.fourcc != VA_FOURCC_NV12) {
+    LOG(ERROR) << "Unexpected image format: " << image.format.fourcc;
+    wrapper_->ReturnVaImageForTesting(&image);
+    return;
+  }
+
+  // Convert NV12 to I420 format.
+  scoped_refptr<media::VideoFrame> frame = CopyNV12ToI420(&image, mem);
+
+  if (frame.get()) {
+    if (!ProcessVideoFrame(frame)) {
+      LOG(ERROR) << "Write to file failed";
+    }
+  } else {
+    LOG(ERROR) << "Cannot convert image to I420.";
+  }
+
+  wrapper_->ReturnVaImageForTesting(&image);
+}
+
+void VaapiH264DecoderLoop::RecycleSurface(VASurfaceID va_surface_id) {
+  available_surfaces_.push_back(va_surface_id);
+}
+
+void VaapiH264DecoderLoop::RefillSurfaces() {
+  for (size_t i = 0; i < available_surfaces_.size(); i++) {
+    VASurface::ReleaseCB release_cb = base::Bind(
+        &VaapiH264DecoderLoop::RecycleSurface, base::Unretained(this));
+    scoped_refptr<VASurface> surface(
+        new VASurface(available_surfaces_[i], release_cb));
+    decoder_->ReuseSurface(surface);
+  }
+  available_surfaces_.clear();
+}
+
+bool VaapiH264DecoderLoop::AllocateNewSurfaces() {
+  CHECK_EQ(num_surfaces_, available_surfaces_.size())
+      << "not all surfaces are returned";
+
+  available_surfaces_.clear();
+  wrapper_->DestroySurfaces();
+
+  gfx::Size size = decoder_->GetPicSize();
+  num_surfaces_ = decoder_->GetRequiredNumOfPictures();
+  return wrapper_->CreateSurfaces(size, num_surfaces_, &available_surfaces_);
+}
+
+TEST(VaapiH264DecoderTest, TestDecode) {
+  base::FilePath input_file = g_input_file;
+  base::FilePath output_file = g_output_file;
+  std::string md5sum = g_md5sum;
+
+  // If nothing specified, use the default file in the source tree.
+  if (input_file.empty() && output_file.empty() && md5sum.empty()) {
+    input_file = base::FilePath(kDefaultInputFile);
+    md5sum = kDefaultMD5Sum;
+  } else {
+    ASSERT_FALSE(input_file.empty()) << "Need to specify --input_file";
+  }
+
+  VLOG(1) << "Input File: " << input_file.value();
+  VLOG(1) << "Output File: " << output_file.value();
+  VLOG(1) << "Expected MD5 sum: " << md5sum;
+
+  content::VaapiH264DecoderLoop loop;
+  ASSERT_TRUE(loop.Initialize(input_file, output_file))
+      << "initialize decoder loop failed";
+  ASSERT_TRUE(loop.Run()) << "run decoder loop failed";
+
+  if (!md5sum.empty()) {
+    std::string actual = loop.GetMD5Sum();
+    VLOG(1) << "Actual MD5 sum: " << actual;
+    EXPECT_EQ(md5sum, actual);
+  }
+}
+
+}  // namespace
+}  // namespace content
+
+int main(int argc, char** argv) {
+  testing::InitGoogleTest(&argc, argv);  // Removes gtest-specific args.
+  CommandLine::Init(argc, argv);
+
+  // Needed to enable DVLOG through --vmodule.
+  logging::LoggingSettings settings;
+  settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG;
+  settings.dcheck_state =
+      logging::ENABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS;
+  CHECK(logging::InitLogging(settings));
+
+  // Process command line.
+  CommandLine* cmd_line = CommandLine::ForCurrentProcess();
+  CHECK(cmd_line);
+
+  CommandLine::SwitchMap switches = cmd_line->GetSwitches();
+  for (CommandLine::SwitchMap::const_iterator it = switches.begin();
+       it != switches.end();
+       ++it) {
+    if (it->first == "input_file") {
+      content::g_input_file = base::FilePath(it->second);
+      continue;
+    }
+    if (it->first == "output_file") {
+      content::g_output_file = base::FilePath(it->second);
+      continue;
+    }
+    if (it->first == "md5sum") {
+      content::g_md5sum = it->second;
+      continue;
+    }
+    if (it->first == "v" || it->first == "vmodule")
+      continue;
+    LOG(FATAL) << "Unexpected switch: " << it->first << ":" << it->second;
+  }
+
+  return RUN_ALL_TESTS();
+}