File video capture device supports MJPEG format

media/capture/video/file_video_capture_device.cc

Index: media/capture/video/file_video_capture_device.cc
diff --git a/media/capture/video/file_video_capture_device.cc b/media/capture/video/file_video_capture_device.cc
index e7d9f064195f00c622f22724bd6eb995e7a0caec..01dca77695452fc37019c0537999760dbe216e18 100644
--- a/media/capture/video/file_video_capture_device.cc
+++ b/media/capture/video/file_video_capture_device.cc
@@ -8,13 +8,77 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "media/base/video_capture_types.h"
+#include "media/filters/jpeg_parser.h"
 
 namespace media {
+
 static const int kY4MHeaderMaxSize = 200;
 static const char kY4MSimpleFrameDelimiter[] = "FRAME";
 static const int kY4MSimpleFrameDelimiterSize = 6;
+static const int kMJpegFrameRate = 30;

[mcasas, 2015/08/19 22:17:35]

float, 30.0f

[henryhsu, 2015/08/20 03:18:56]

Done.

+
+FileVideoCaptureDevice::VideoFileParser::VideoFileParser(
+    const base::FilePath& file_path)
+    : file_path_(file_path),
+      frame_size_(0),
+      current_byte_index_(0),
+      first_frame_byte_index_(0) {}
+
+FileVideoCaptureDevice::VideoFileParser::~VideoFileParser() {}
+
+FileVideoCaptureDevice::Y4mFileParser::Y4mFileParser(
+    const base::FilePath& file_path)
+    : VideoFileParser(file_path) {}
+
+FileVideoCaptureDevice::Y4mFileParser::~Y4mFileParser() {}
+
+bool FileVideoCaptureDevice::Y4mFileParser::Initialize(
+    media::VideoCaptureFormat* capture_format) {
+  file_.reset(new base::File(file_path_,
+                             base::File::FLAG_OPEN | base::File::FLAG_READ));
+  if (!file_->IsValid()) {
+    DLOG(ERROR) << file_path_.value() << ", error: "
+                << base::File::ErrorToString(file_->error_details());
+    return false;
+  }
+
+  std::string header(kY4MHeaderMaxSize, 0);

[kcwu, 2015/08/19 11:29:38]

s/0/'\0'/

[henryhsu, 2015/08/20 03:18:57]

Done.

+  file_->Read(0, &header[0], kY4MHeaderMaxSize - 1);

[kcwu, 2015/08/19 11:29:38]

s/kY4MHeaderMaxSize - 1/header.size()/

[henryhsu, 2015/08/20 03:18:57]

Done.

+  const size_t header_end = header.find(kY4MSimpleFrameDelimiter);
+  CHECK_NE(header_end, header.npos);
+
+  ParseY4MTags(header, capture_format);
+  first_frame_byte_index_ = header_end + kY4MSimpleFrameDelimiterSize;
+  current_byte_index_ = first_frame_byte_index_;
+  frame_size_ = capture_format->ImageAllocationSize();
+  video_frame_.reset(new uint8[frame_size_]);

[kcwu, 2015/08/19 11:29:38]

uint8_t

[henryhsu, 2015/08/20 03:18:57]

Done.

+  return true;
+}
+
+const uint8_t* FileVideoCaptureDevice::Y4mFileParser::GetNextFrame(
+    int* frame_size) {
+  int result =
+      file_->Read(current_byte_index_,
+                  reinterpret_cast<char*>(video_frame_.get()), frame_size_);
 
-int ParseY4MInt(const base::StringPiece& token) {
+  // If we passed EOF to base::File, it will return 0 read characters. In that
+  // case, reset the pointer andd read again.

[kcwu, 2015/08/19 11:29:38]

s/andd/and/

[henryhsu, 2015/08/20 03:18:57]

Done.

+  if (result != frame_size_) {
+    CHECK_EQ(result, 0);
+    current_byte_index_ = first_frame_byte_index_;
+    CHECK_EQ(
+        file_->Read(current_byte_index_,
+                    reinterpret_cast<char*>(video_frame_.get()), frame_size_),
+        frame_size_);
+  } else {
+    current_byte_index_ += frame_size_ + kY4MSimpleFrameDelimiterSize;
+  }
+  *frame_size = frame_size_;
+  return video_frame_.get();
+}
+
+int FileVideoCaptureDevice::Y4mFileParser::ParseY4MInt(

[mcasas, 2015/08/19 22:17:34]

This method should be a file function as before.

[henryhsu, 2015/08/20 03:18:56]

Done.

+    const base::StringPiece& token) {
   int temp_int;
   CHECK(base::StringToInt(token, &temp_int)) << token;
   return temp_int;
@@ -22,9 +86,10 @@ int ParseY4MInt(const base::StringPiece& token) {
 
 // Extract numerator and denominator out of a token that must have the aspect
 // numerator:denominator, both integer numbers.
-void ParseY4MRational(const base::StringPiece& token,
-                      int* numerator,
-                      int* denominator) {
+void FileVideoCaptureDevice::Y4mFileParser::ParseY4MRational(

[mcasas, 2015/08/19 22:17:34]

Same: This method should be a file function as before.

[henryhsu, 2015/08/20 03:18:57]

Done.

+    const base::StringPiece& token,
+    int* numerator,
+    int* denominator) {
   size_t index_divider = token.find(':');
   CHECK_NE(index_divider, token.npos);
   *numerator = ParseY4MInt(token.substr(0, index_divider));
@@ -43,11 +108,11 @@ void ParseY4MRational(const base::StringPiece& token,
 // with a newline character instead. Also, some headers do _not_ specify pixel
 // format, in this case it means I420.
 // This code was inspired by third_party/libvpx/.../y4minput.* .
-void ParseY4MTags(const std::string& file_header,
-                  media::VideoCaptureFormat* video_format) {
-  video_format->pixel_format = media::VIDEO_CAPTURE_PIXEL_FORMAT_I420;
-  video_format->frame_size.set_width(0);
-  video_format->frame_size.set_height(0);
+void FileVideoCaptureDevice::Y4mFileParser::ParseY4MTags(

[mcasas, 2015/08/19 22:17:34]

Same: This method should be a file function as before.

[henryhsu, 2015/08/20 03:18:57]

Done.

+    const std::string& file_header,
+    media::VideoCaptureFormat* video_format) {
+  media::VideoCaptureFormat format;
+  format.pixel_format = media::VIDEO_CAPTURE_PIXEL_FORMAT_I420;
   size_t index = 0;
   size_t blank_position = 0;
   base::StringPiece token;
@@ -60,10 +125,10 @@ void ParseY4MTags(const std::string& file_header,
     CHECK(!token.empty());
     switch (file_header[index]) {
       case 'W':
-        video_format->frame_size.set_width(ParseY4MInt(token));
+        format.frame_size.set_width(ParseY4MInt(token));
         break;
       case 'H':
-        video_format->frame_size.set_height(ParseY4MInt(token));
+        format.frame_size.set_height(ParseY4MInt(token));
         break;
       case 'F': {
         // If the token is "FRAME", it means we have finished with the header.
@@ -71,7 +136,7 @@ void ParseY4MTags(const std::string& file_header,
           break;
         int fps_numerator, fps_denominator;
         ParseY4MRational(token, &fps_numerator, &fps_denominator);
-        video_format->frame_rate = fps_numerator / fps_denominator;
+        format.frame_rate = fps_numerator / fps_denominator;
         break;
       }
       case 'I':
@@ -84,7 +149,6 @@ void ParseY4MTags(const std::string& file_header,
       case 'C':
         CHECK(token == "420" || token == "420jpeg" || token == "420paldv")
             << token;  // Only I420 is supported, and we fudge the variants.
-        break;
       default:

[kcwu, 2015/08/19 11:29:38]

why removed break; above this line?

[henryhsu, 2015/08/20 03:18:56]

Thanks.

         break;
     }
@@ -94,47 +158,101 @@ void ParseY4MTags(const std::string& file_header,
     index = blank_position + 1;
   }
   // Last video format semantic correctness check before sending it back.
-  CHECK(video_format->IsValid());
+  CHECK(format.IsValid());
+  *video_format = format;
 }
 
-// Reads and parses the header of a Y4M |file|, returning the collected pixel
-// format in |video_format|. Returns the index of the first byte of the first
-// video frame.
-// Restrictions: Only trivial per-frame headers are supported.
-// static
-int64 FileVideoCaptureDevice::ParseFileAndExtractVideoFormat(
-    base::File* file,
-    media::VideoCaptureFormat* video_format) {
-  std::string header(kY4MHeaderMaxSize, 0);
-  file->Read(0, &header[0], kY4MHeaderMaxSize - 1);
+FileVideoCaptureDevice::MjpegFileParser::MjpegFileParser(
+    const base::FilePath& file_path)
+    : VideoFileParser(file_path) {}
 
-  size_t header_end = header.find(kY4MSimpleFrameDelimiter);
-  CHECK_NE(header_end, header.npos);
+FileVideoCaptureDevice::MjpegFileParser::~MjpegFileParser() {}
+
+bool FileVideoCaptureDevice::MjpegFileParser::Initialize(
+    media::VideoCaptureFormat* capture_format) {
+  mapped_file_.reset(new base::MemoryMappedFile());
+
+  if (!mapped_file_->Initialize(file_path_) || !mapped_file_->IsValid()) {
+    LOG(ERROR) << "File memory map error: " << file_path_.value();
+    return false;
+  }
+
+  JpegParseResult result;
+  if (!ParseJpegPicture(mapped_file_->data(), mapped_file_->length(), &result))
+    return false;
 
-  ParseY4MTags(header, video_format);
-  return header_end + kY4MSimpleFrameDelimiterSize;
+  frame_size_ = result.image_size;
+  if (frame_size_ > static_cast<int>(mapped_file_->length())) {
+    LOG(ERROR) << "File is incomplete";
+    return false;
+  }
+
+  VideoCaptureFormat format;
+  format.pixel_format = media::VIDEO_CAPTURE_PIXEL_FORMAT_MJPEG;
+  format.frame_size.set_width(result.frame_header.visible_width);
+  format.frame_size.set_height(result.frame_header.visible_height);
+  format.frame_rate = kMJpegFrameRate;
+  if (!format.IsValid())
+    return false;
+  *capture_format = format;
+  return true;
+}
+
+const uint8_t* FileVideoCaptureDevice::MjpegFileParser::GetNextFrame(
+    int* frame_size) {
+  const uint8_t* buf_ptr = mapped_file_->data() + current_byte_index_;
+
+  JpegParseResult result;
+  if (!ParseJpegPicture(buf_ptr, mapped_file_->length() - current_byte_index_,
+                        &result)) {
+    return nullptr;
+  }
+  *frame_size = frame_size_ = result.image_size;

[mcasas, 2015/08/19 22:17:35]

Should we DCHECK something of |result|?
DCHECK_GT(0u, result.data_size); ?

[henryhsu, 2015/08/20 03:18:57]

We don't use result.data_size. I think if ParseJpegPicture returns true, we can
use the result directly without check. All error cases will cause
ParseJpegPicture return false.

+  current_byte_index_ += frame_size_;
+  // reset the pointer to play repeatedly.

[mcasas, 2015/08/19 22:17:34]

s/reset/Reset/

[henryhsu, 2015/08/20 03:18:57]

Done.

+  if (current_byte_index_ >= mapped_file_->length())
+    current_byte_index_ = first_frame_byte_index_;
+  return buf_ptr;
 }
 
-// Opens a given file for reading, and returns the file to the caller, who is
-// responsible for closing it.
 // static
-base::File FileVideoCaptureDevice::OpenFileForRead(
-    const base::FilePath& file_path) {
-  base::File file(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ);
-  DLOG_IF(ERROR, file.IsValid())
-      << file_path.value()
-      << ", error: " << base::File::ErrorToString(file.error_details());
-  return file.Pass();
+bool FileVideoCaptureDevice::GetVideoCaptureFormat(
+    const base::FilePath& file_path,
+    media::VideoCaptureFormat* video_format) {
+  scoped_ptr<VideoFileParser> file_parser =
+      GetVideoFileParser(file_path, video_format);

[mcasas, 2015/08/19 22:17:34]

Creating a VideoFileParser just for checking
if the file extension is ".y4m" or ".mjpeg"
and parsing its header seems a bit of a waste
(we allocate frame-size buffers). Please have 
another go at it.

[henryhsu, 2015/08/20 03:18:57]

I move buffer allocation to GetNextFrame.
Only allocate the buffer when we need.

+  if (!file_parser)

[kcwu, 2015/08/19 11:29:38]

return file_parser != nullptr;

[henryhsu, 2015/08/20 03:18:57]

Done.

+    return false;
+  return true;
 }
 
-FileVideoCaptureDevice::FileVideoCaptureDevice(const base::FilePath& file_path)
-    : capture_thread_("CaptureThread"),
-      file_path_(file_path),
-      frame_size_(0),
-      current_byte_index_(0),
-      first_frame_byte_index_(0) {
+// static
+scoped_ptr<FileVideoCaptureDevice::VideoFileParser>
+FileVideoCaptureDevice::GetVideoFileParser(
+    const base::FilePath& file_path,
+    media::VideoCaptureFormat* video_format) {
+  scoped_ptr<VideoFileParser> file_parser;
+
+  if (base::EndsWith(file_path.value(), "y4m",
+                     base::CompareCase::INSENSITIVE_ASCII)) {
+    file_parser.reset(new Y4mFileParser(file_path));
+  } else if (base::EndsWith(file_path.value(), "mjpeg",
+                            base::CompareCase::INSENSITIVE_ASCII)) {
+    file_parser.reset(new MjpegFileParser(file_path));
+  } else {
+    LOG(ERROR) << "Unsupported file format.";
+    return file_parser.Pass();
+  }
+
+  if (!file_parser || !file_parser->Initialize(video_format)) {
+    file_parser.reset();
+  }
+  return file_parser.Pass();
 }
 
+FileVideoCaptureDevice::FileVideoCaptureDevice(const base::FilePath& file_path)
+    : capture_thread_("CaptureThread"), file_path_(file_path) {}
+
 FileVideoCaptureDevice::~FileVideoCaptureDevice() {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Check if the thread is running.
@@ -165,12 +283,6 @@ void FileVideoCaptureDevice::StopAndDeAllocate() {
   capture_thread_.Stop();
 }
 
-int FileVideoCaptureDevice::CalculateFrameSize() const {
-  DCHECK_EQ(capture_format_.pixel_format, VIDEO_CAPTURE_PIXEL_FORMAT_I420);
-  DCHECK_EQ(capture_thread_.message_loop(), base::MessageLoop::current());
-  return capture_format_.ImageAllocationSize();
-}
-
 void FileVideoCaptureDevice::OnAllocateAndStart(
     const VideoCaptureParams& params,
     scoped_ptr<VideoCaptureDevice::Client> client) {
@@ -178,22 +290,16 @@ void FileVideoCaptureDevice::OnAllocateAndStart(
 
   client_ = client.Pass();
 
-  // Open the file and parse the header. Get frame size and format.
-  DCHECK(!file_.IsValid());
-  file_ = OpenFileForRead(file_path_);
-  if (!file_.IsValid()) {
+  DCHECK(!file_parser_);
+  file_parser_ = GetVideoFileParser(file_path_, &capture_format_);
+  if (!file_parser_) {
     client_->OnError("Could not open Video file");
     return;
   }
-  first_frame_byte_index_ =
-      ParseFileAndExtractVideoFormat(&file_, &capture_format_);
-  current_byte_index_ = first_frame_byte_index_;
+
   DVLOG(1) << "Opened video file " << capture_format_.frame_size.ToString()
            << ", fps: " << capture_format_.frame_rate;
 
-  frame_size_ = CalculateFrameSize();
-  video_frame_.reset(new uint8[frame_size_]);
-
   capture_thread_.message_loop()->PostTask(
       FROM_HERE, base::Bind(&FileVideoCaptureDevice::OnCaptureTask,
                             base::Unretained(this)));
@@ -201,40 +307,23 @@ void FileVideoCaptureDevice::OnAllocateAndStart(
 
 void FileVideoCaptureDevice::OnStopAndDeAllocate() {
   DCHECK_EQ(capture_thread_.message_loop(), base::MessageLoop::current());
-  file_.Close();
+  file_parser_.reset();
   client_.reset();
-  current_byte_index_ = 0;
-  first_frame_byte_index_ = 0;
-  frame_size_ = 0;
   next_frame_time_ = base::TimeTicks();
-  video_frame_.reset();
 }
 
 void FileVideoCaptureDevice::OnCaptureTask() {
   DCHECK_EQ(capture_thread_.message_loop(), base::MessageLoop::current());
   if (!client_)
     return;
-  int result =
-      file_.Read(current_byte_index_,
-                 reinterpret_cast<char*>(video_frame_.get()), frame_size_);
-
-  // If we passed EOF to base::File, it will return 0 read characters. In that
-  // case, reset the pointer and read again.
-  if (result != frame_size_) {
-    CHECK_EQ(result, 0);
-    current_byte_index_ = first_frame_byte_index_;
-    CHECK_EQ(
-        file_.Read(current_byte_index_,
-                   reinterpret_cast<char*>(video_frame_.get()), frame_size_),
-        frame_size_);
-  } else {
-    current_byte_index_ += frame_size_ + kY4MSimpleFrameDelimiterSize;
-  }
 
   // Give the captured frame to the client.
+  int frame_size = 0;
+  const uint8_t* frame_ptr = file_parser_->GetNextFrame(&frame_size);
+  CHECK(frame_ptr);

[mcasas, 2015/08/19 22:17:35]

and DCHECK(frame_size); ?

It might look like overly zealous, but after 
implementing the first FileVCD I got all kinds 
of feedback, and most of the complains were
easily traceable to strange/erroneous/bizarre
files, hence the DCHECK()s everywhere -- so
smart developers solve their own problems ISO
filing bugs :)

[henryhsu, 2015/08/20 03:18:57]

Done.

   const base::TimeTicks current_time = base::TimeTicks::Now();
-  client_->OnIncomingCapturedData(video_frame_.get(), frame_size_,
-                                  capture_format_, 0, current_time);
+  client_->OnIncomingCapturedData(frame_ptr, frame_size, capture_format_, 0,
+                                  current_time);
   // Reschedule next CaptureTask.
   const base::TimeDelta frame_interval =
       base::TimeDelta::FromMicroseconds(1E6 / capture_format_.frame_rate);