Media Galleries API Metadata: Add audio video metadata extractor.

chrome/utility/media_galleries/media_metadata_parser.cc

Index: chrome/utility/media_galleries/media_metadata_parser.cc
diff --git a/chrome/utility/media_galleries/media_metadata_parser.cc b/chrome/utility/media_galleries/media_metadata_parser.cc
index 33102c9c6cf3368c74d39c2c34983bfce0e11eba..da0d669ebb8940b350c9d9e1b6a9363c62f4cd29 100644
--- a/chrome/utility/media_galleries/media_metadata_parser.cc
+++ b/chrome/utility/media_galleries/media_metadata_parser.cc
@@ -4,14 +4,77 @@
 
 #include "chrome/utility/media_galleries/media_metadata_parser.h"
 
+#include <map>
+#include <string>
+
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "media/base/audio_video_metadata_extractor.h"
+#include "media/base/data_source.h"
 
 namespace metadata {
 
-MediaMetadataParser::MediaMetadataParser(DataReader* reader,
+namespace {
+
+void SetStringScopedPtrFromDict(const std::map<std::string, std::string>& dict,
+                                const std::string& key,
+                                scoped_ptr<std::string>* result) {
+  DCHECK(result);
+  std::map<std::string, std::string>::const_iterator it = dict.find(key);
+  if (it == dict.end())
+    return;
+
+  result->reset(new std::string(it->second));
+}
+
+void SetIntScopedPtrFromDict(const std::map<std::string, std::string>& dict,
+                             const std::string& key,
+                             scoped_ptr<int>* result) {
+  DCHECK(result);
+  std::map<std::string, std::string>::const_iterator it = dict.find(key);
+  if (it == dict.end())
+    return;
+
+  int converted;
+  if (base::StringToInt(it->second, &converted))
+    result->reset(new int(converted));
+}
+
+void PopulateAudioVideoMetadata(media::DataSource* source,
+                                MediaMetadataParser::MediaMetadata* metadata) {
+  DCHECK(source);
+  DCHECK(metadata);
+  media::AudioVideoMetadataExtractor extractor(source);
+
+  if (!extractor.Extract())
+    return;
+
+  metadata->duration_seconds.reset(new int(extractor.GetDurationSeconds()));
+
+  if (extractor.GetHeight() > 0 && extractor.GetWidth() > 0) {
+    metadata->height.reset(new int(extractor.GetHeight()));
+    metadata->width.reset(new int(extractor.GetWidth()));
+  }
+
+  const std::map<std::string, std::string>& tags = extractor.GetTags();
+  SetStringScopedPtrFromDict(tags, "album", &metadata->album);

[acolwell GONE FROM CHROMIUM, 2014/01/03 19:14:04]

This seems brittle and unnecessarily dependent on FFmpeg's choice of tag names.
I think the extractor should have explicit methods for each of these pieces of
data like you have for the width and height. That way only the extractor has to
care about FFmpeg details.

[tommycli, 2014/01/03 23:08:36]

Done.

+  SetStringScopedPtrFromDict(tags, "artist", &metadata->artist);
+  SetStringScopedPtrFromDict(tags, "comment", &metadata->comment);
+  SetStringScopedPtrFromDict(tags, "copyright", &metadata->copyright);
+  SetIntScopedPtrFromDict(tags, "disc", &metadata->disc);
+  SetStringScopedPtrFromDict(tags, "genre", &metadata->genre);
+  SetStringScopedPtrFromDict(tags, "language", &metadata->language);
+  SetStringScopedPtrFromDict(tags, "title", &metadata->title);
+  SetIntScopedPtrFromDict(tags, "track", &metadata->track);
+}
+
+}  // namespace
+
+MediaMetadataParser::MediaMetadataParser(media::DataSource* source,
                                          const std::string& mime_type)
-    : reader_(reader),
+    : source_(source),
       metadata_(new MediaMetadata) {
   metadata_->mime_type = mime_type;
 }
@@ -22,7 +85,12 @@ void MediaMetadataParser::Start(const MetadataCallback& callback) {
   DCHECK(callback_.is_null());
   callback_ = callback;
 
-  // TODO(tommycli): Implement for various mime types.
+  if (StartsWithASCII(metadata_->mime_type, "audio/", true) ||
+      StartsWithASCII(metadata_->mime_type, "video/", true)) {
+    PopulateAudioVideoMetadata(source_.get(), metadata_.get());

[acolwell GONE FROM CHROMIUM, 2014/01/03 19:14:04]

nit: Why don't you simply make this a private member function so you don't have
to pass in these members?

[tommycli, 2014/01/03 23:08:36]

Done.

+  }
+
+  // TODO(tommycli): Implement for image mime types.
   callback_.Run(metadata_.Pass());
 }