Check for sample aspect ratio, frame duration, and time scale

- These fields are required to generate DASH IOP compliant MPDs
  of type=video.

Change-Id: I142ecd662e454ae10d06d66aa5519171f5995303

packager/mpd/base/media_info.proto

@@ -20,13 +20,25 @@ message MediaInfo {
 
   message VideoInfo {
     optional string codec = 1;
+
+    // The width and height of the actual number of pixels. This will not be the
+    // same as the visual width and height if the sample aspect ratio (sar)
+    // is not 1:1.
     optional uint32 width = 2;
     optional uint32 height = 3;
+
     optional uint32 time_scale = 4;
     // Relative to |time_scale|. IOW |time_scale| / |frame_duration| is the
     // framerate.
     optional uint64 frame_duration = 5;
     optional bytes decoder_config = 6;
+
+    // pixel_width:pixel_height is the the sample aspect ratio (sar) of the
+    // video.
+    // Note that (pixel_width * width):(pixel_height * height) is the picture
+    // aspect ratio, or the @par attribute set on AdaptationSet element.
+    optional uint32 pixel_width = 7;
+    optional uint32 pixel_height = 8;
   }
 
   message AudioInfo {

packager/mpd/base/mpd_builder.cc

@@ -35,8 +35,7 @@ using xml::AdaptationSetXmlNode;
 
 namespace {
 
-std::string GetMimeType(
+std::string GetMimeType(const std::string& prefix,
-    const std::string& prefix,
                         MediaInfo::ContainerType container_type) {
   switch (container_type) {
     case MediaInfo::CONTAINER_MP4:
@@ -60,7 +59,8 @@ void AddMpdNameSpaceInfo(XmlNode* mpd) {
 
   static const char kXmlNamespace[] = "urn:mpeg:DASH:schema:MPD:2011";
   mpd->SetStringAttribute("xmlns", kXmlNamespace);
-  static const char kXmlNamespaceXsi[] = "http://www.w3.org/2001/XMLSchema-instance";
+  static const char kXmlNamespaceXsi[] =
+      "http://www.w3.org/2001/XMLSchema-instance";
   mpd->SetStringAttribute("xmlns:xsi", kXmlNamespaceXsi);
   static const char kXmlNamespaceXlink[] = "http://www.w3.org/1999/xlink";
   mpd->SetStringAttribute("xmlns:xlink", kXmlNamespaceXlink);
@@ -82,8 +82,7 @@ bool IsPeriodNode(xmlNodePtr node) {
 // As noted here, we must traverse.
 // http://www.xmlsoft.org/tutorial/ar01s04.html
 xmlNodePtr FindPeriodNode(XmlNode* xml_node) {
-  for (xmlNodePtr node = xml_node->GetRawPtr()->xmlChildrenNode;
+  for (xmlNodePtr node = xml_node->GetRawPtr()->xmlChildrenNode; node != NULL;
-       node != NULL;
        node = node->next) {
     if (IsPeriodNode(node))
       return node;
@@ -103,12 +102,9 @@ std::string XmlDateTimeNowWithOffset(int32_t offset_seconds) {
   base::Time::Exploded time_exploded;
   time.UTCExplode(&time_exploded);
 
-  return base::StringPrintf("%4d-%02d-%02dT%02d:%02d:%02d",
+  return base::StringPrintf("%4d-%02d-%02dT%02d:%02d:%02d", time_exploded.year,
-                            time_exploded.year,
+                            time_exploded.month, time_exploded.day_of_month,
-                            time_exploded.month,
+                            time_exploded.hour, time_exploded.minute,
-                            time_exploded.day_of_month,
-                            time_exploded.hour,
-                            time_exploded.minute,
                             time_exploded.second);
 }
 
@@ -185,10 +181,42 @@ bool WriteXmlCharArrayToOutput(xmlChar* doc,
   return output->Flush();
 }
 
-std::string MakePathRelative(const std::string& path, const std::string& mpd_dir) {
+std::string MakePathRelative(const std::string& path,
+                             const std::string& mpd_dir) {
   return (path.find(mpd_dir) == 0) ? path.substr(mpd_dir.size()) : path;
 }
 
+// Check whether all the video infos have width and height.
+// DASH IOP defines required fields for video representations, namely
+// width, height, framerate, and sar.
+bool HasRequiredVideoFields(
+    ::google::protobuf::RepeatedPtrField<MediaInfo_VideoInfo> video_infos) {
+  CHECK_GT(video_infos.size(), 0);
+  for (int i = 0; i < video_infos.size(); ++i) {
+    const MediaInfo::VideoInfo& info = video_infos.Get(i);
+    if (!info.has_height() || !info.has_width()) {
+      LOG(ERROR)
+          << "Width and height are required fields for generating a valid MPD.";
+      return false;
+    }
+    // These fields are not required for a valid MPD, but required for DASH IOP
+    // compliant MPD. MpdBuilder can keep generating MPDs without these fields.
+    LOG_IF(WARNING, !info.has_time_scale())
+        << "Video info does not contain timescale required for "
+           "calculating framerate. @frameRate is required for DASH IOP.";
+    LOG_IF(WARNING, !info.has_frame_duration())
+        << "Video info does not contain frame duration required "
+           "for calculating framerate. @frameRate is required for DASH IOP.";
+    LOG_IF(WARNING, !info.has_pixel_width())
+        << "Video info does not contain pixel_width to calculate the sample "
+           "aspect ratio required for DASH IOP.";
+    LOG_IF(WARNING, !info.has_pixel_height())
+        << "Video info does not contain pixel_height to calculate the sample "
+           "aspect ratio required for DASH IOP.";
+  }
+  return true;
+}
+
 // Spooky static initialization/cleanup of libxml.
 class LibXmlInitializer {
  public:
@@ -220,9 +248,11 @@ class LibXmlInitializer {
 MpdBuilder::MpdBuilder(MpdType type, const MpdOptions& mpd_options)
     : type_(type),
       mpd_options_(mpd_options),
-      adaptation_sets_deleter_(&adaptation_sets_) {}
+      adaptation_sets_deleter_(&adaptation_sets_) {
+}
 
-MpdBuilder::~MpdBuilder() {}
+MpdBuilder::~MpdBuilder() {
+}
 
 void MpdBuilder::AddBaseUrl(const std::string& base_url) {
   base::AutoLock scoped_lock(lock_);
@@ -231,8 +261,8 @@ void MpdBuilder::AddBaseUrl(const std::string& base_url) {
 
 AdaptationSet* MpdBuilder::AddAdaptationSet(const std::string& lang) {
   base::AutoLock scoped_lock(lock_);
-  scoped_ptr<AdaptationSet> adaptation_set(new AdaptationSet(
+  scoped_ptr<AdaptationSet> adaptation_set(
-      adaptation_set_counter_.GetNext(), lang, mpd_options_,
+      new AdaptationSet(adaptation_set_counter_.GetNext(), lang, mpd_options_,
                         &representation_counter_));
 
   DCHECK(adaptation_set);
@@ -263,8 +293,8 @@ bool MpdBuilder::WriteMpdToOutput(OutputType* output) {
   static const int kNiceFormat = 1;
   int doc_str_size = 0;
   xmlChar* doc_str = NULL;
-  xmlDocDumpFormatMemoryEnc(
+  xmlDocDumpFormatMemoryEnc(doc.get(), &doc_str, &doc_str_size, "UTF-8",
-      doc.get(), &doc_str, &doc_str_size, "UTF-8", kNiceFormat);
+                            kNiceFormat);
 
   bool result = WriteXmlCharArrayToOutput(doc_str, doc_str_size, output);
   xmlFree(doc_str);
@@ -330,8 +360,7 @@ xmlDocPtr MpdBuilder::GenerateMpd() {
 void MpdBuilder::AddCommonMpdInfo(XmlNode* mpd_node) {
   if (Positive(mpd_options_.min_buffer_time)) {
     mpd_node->SetStringAttribute(
-        "minBufferTime",
+        "minBufferTime", SecondsToXmlDuration(mpd_options_.min_buffer_time));
-        SecondsToXmlDuration(mpd_options_.min_buffer_time));
   } else {
     LOG(ERROR) << "minBufferTime value not specified.";
     // TODO(tinskip): Propagate error.
@@ -368,8 +397,8 @@ void MpdBuilder::AddDynamicMpdInfo(XmlNode* mpd_node) {
     double earliest_presentation_time;
     if (GetEarliestTimestamp(&earliest_presentation_time)) {
       availability_start_time_ =
-          XmlDateTimeNowWithOffset(mpd_options_.availability_time_offset
+          XmlDateTimeNowWithOffset(mpd_options_.availability_time_offset -
-                                   - std::ceil(earliest_presentation_time));
+                                   std::ceil(earliest_presentation_time));
     } else {
       LOG(ERROR) << "Could not determine the earliest segment presentation "
                     "time for availabilityStartTime calculation.";
@@ -377,7 +406,8 @@ void MpdBuilder::AddDynamicMpdInfo(XmlNode* mpd_node) {
     }
   }
   if (!availability_start_time_.empty())
-    mpd_node->SetStringAttribute("availabilityStartTime", availability_start_time_);
+    mpd_node->SetStringAttribute("availabilityStartTime",
+                                 availability_start_time_);
 
   if (Positive(mpd_options_.minimum_update_period)) {
     mpd_node->SetStringAttribute(
@@ -388,11 +418,10 @@ void MpdBuilder::AddDynamicMpdInfo(XmlNode* mpd_node) {
                     "specified.";
   }
 
-  SetIfPositive(
+  SetIfPositive("timeShiftBufferDepth", mpd_options_.time_shift_buffer_depth,
-      "timeShiftBufferDepth", mpd_options_.time_shift_buffer_depth, mpd_node);
-  SetIfPositive("suggestedPresentationDelay",
-                mpd_options_.suggested_presentation_delay,
                 mpd_node);
+  SetIfPositive("suggestedPresentationDelay",
+                mpd_options_.suggested_presentation_delay, mpd_node);
 }
 
 float MpdBuilder::GetStaticMpdDuration(XmlNode* mpd_node) {
@@ -408,8 +437,7 @@ float MpdBuilder::GetStaticMpdDuration(XmlNode* mpd_node) {
   // attribute.
   float max_duration = 0.0f;
   for (xmlNodePtr adaptation_set = xmlFirstElementChild(period_node);
-       adaptation_set;
+       adaptation_set; adaptation_set = xmlNextElementSibling(adaptation_set)) {
-       adaptation_set = xmlNextElementSibling(adaptation_set)) {
     for (xmlNodePtr representation = xmlFirstElementChild(adaptation_set);
          representation;
          representation = xmlNextElementSibling(representation)) {
@@ -433,8 +461,7 @@ bool MpdBuilder::GetEarliestTimestamp(double* timestamp_seconds) {
   double earliest_timestamp(-1);
   for (std::list<AdaptationSet*>::const_iterator iter =
            adaptation_sets_.begin();
-       iter != adaptation_sets_.end();
+       iter != adaptation_sets_.end(); ++iter) {
-       ++iter) {
     double timestamp;
     if ((*iter)->GetEarliestTimestamp(&timestamp) &&
         ((earliest_timestamp < 0) || (timestamp < earliest_timestamp))) {
@@ -452,8 +479,9 @@ void MpdBuilder::MakePathsRelativeToMpd(const std::string& mpd_path,
                                         MediaInfo* media_info) {
   DCHECK(media_info);
   const std::string kFileProtocol("file://");
-  std::string mpd_file_path = (mpd_path.find(kFileProtocol) == 0) ?
+  std::string mpd_file_path = (mpd_path.find(kFileProtocol) == 0)
-      mpd_path.substr(kFileProtocol.size()) : mpd_path;
+                                  ? mpd_path.substr(kFileProtocol.size())
+                                  : mpd_path;
 
   if (!mpd_file_path.empty()) {
     std::string mpd_dir(
@@ -487,7 +515,8 @@ AdaptationSet::AdaptationSet(uint32_t adaptation_set_id,
   DCHECK(counter);
 }
 
-AdaptationSet::~AdaptationSet() {}
+AdaptationSet::~AdaptationSet() {
+}
 
 Representation* AdaptationSet::AddRepresentation(const MediaInfo& media_info) {
   base::AutoLock scoped_lock(lock_);
@@ -542,8 +571,7 @@ bool AdaptationSet::GetEarliestTimestamp(double* timestamp_seconds) {
   double earliest_timestamp(-1);
   for (std::list<Representation*>::const_iterator iter =
            representations_.begin();
-       iter != representations_.end();
+       iter != representations_.end(); ++iter) {
-       ++iter) {
     double timestamp;
     if ((*iter)->GetEarliestTimestamp(&timestamp) &&
         ((earliest_timestamp < 0) || (timestamp < earliest_timestamp))) {
@@ -567,7 +595,8 @@ Representation::Representation(const MediaInfo& media_info,
       start_number_(1) {
 }
 
-Representation::~Representation() {}
+Representation::~Representation() {
+}
 
 bool Representation::Init() {
   codecs_ = GetCodecs(media_info_);
@@ -592,10 +621,14 @@ bool Representation::Init() {
     return false;
   }
 
-  // Check video and then audio. Usually when there is audio + video, we take
+  // For mimetypes, this checks the video and then audio. Usually when there is
-  // video/<type>.
+  // audio + video, we take video/<type>.
   if (has_video_info) {
     mime_type_ = GetVideoMimeType();
+    if (!HasRequiredVideoFields(media_info_.video_info())) {
+      LOG(ERROR) << "Missing required fields to create a video Representation.";
+      return false;
+    }
   } else if (has_audio_info) {
     mime_type_ = GetAudioMimeType();
   }
@@ -689,8 +722,8 @@ xml::ScopedXmlPtr<xmlNode>::type Representation::GetXml() {
   }
 
   if (HasLiveOnlyFields(media_info_) &&
-      !representation.AddLiveOnlyInfo(
+      !representation.AddLiveOnlyInfo(media_info_, segment_infos_,
-          media_info_, segment_infos_, start_number_)) {
+                                      start_number_)) {
     LOG(ERROR) << "Failed to add Live info.";
     return xml::ScopedXmlPtr<xmlNode>::type();
   }
@@ -744,9 +777,9 @@ bool Representation::IsContiguous(uint64_t start_time,
       previous.start_time + previous.duration * previous.repeat;
   if (previous_segment_start_time >= start_time) {
     LOG(ERROR) << "Segments should not be out of order segment. Adding segment "
-                  "with start_time == " << start_time
+                  "with start_time == "
-               << " but the previous segment starts at " << previous.start_time
+               << start_time << " but the previous segment starts at "
-               << ".";
+               << previous.start_time << ".";
     return false;
   }
 
@@ -841,8 +874,7 @@ bool Representation::GetEarliestTimestamp(double* timestamp_seconds) {
   if (segment_infos_.empty())
     return false;
 
-  *timestamp_seconds =
+  *timestamp_seconds = static_cast<double>(segment_infos_.begin()->start_time) /
-      static_cast<double>(segment_infos_.begin()->start_time) /
                        GetTimeScale(media_info_);
   return true;
 }

packager/mpd/base/mpd_builder.h

@@ -178,7 +178,7 @@ class AdaptationSet {
 
  private:
   friend class MpdBuilder;
-  FRIEND_TEST_ALL_PREFIXES(StaticMpdBuilderTest, CheckAdaptationSetId);
+  FRIEND_TEST_ALL_PREFIXES(CommonMpdBuilderTest, CheckAdaptationSetId);
 
   /// @param adaptation_set_id is an ID number for this AdaptationSet.
   /// @param representation_counter is a Counter for assigning ID numbers to
@@ -241,7 +241,12 @@ class Representation {
 
  private:
   friend class AdaptationSet;
-  FRIEND_TEST_ALL_PREFIXES(StaticMpdBuilderTest, CheckRepresentationId);
+  // TODO(rkuroiwa): Consider defining a public factory method that constructs
+  // and Init()s, at least for testing.
+  FRIEND_TEST_ALL_PREFIXES(CommonMpdBuilderTest, ValidMediaInfo);
+  FRIEND_TEST_ALL_PREFIXES(CommonMpdBuilderTest, InvalidMediaInfo);
+  FRIEND_TEST_ALL_PREFIXES(CommonMpdBuilderTest, CheckVideoInfoReflectedInXml);
+  FRIEND_TEST_ALL_PREFIXES(CommonMpdBuilderTest, CheckRepresentationId);
 
   /// @param media_info is a MediaInfo containing information on the media.
   ///        @a media_info.bandwidth is required for 'static' profile. If @a

packager/mpd/base/mpd_builder_unittest.cc

@@ -8,6 +8,7 @@
 #include <inttypes.h>
 #include <libxml/xmlstring.h>
 
+#include "base/strings/string_piece.h"
 #include "packager/base/file_util.h"
 #include "packager/base/logging.h"
 #include "packager/base/strings/string_number_conversions.h"
@@ -24,6 +25,9 @@ namespace edash_packager {
 using base::FilePath;
 
 namespace {
+// Any number for RepresentationId. Required to create a Representation but
+// not checked in test.
+const uint32_t kAnyRepresentationId = 1;
 const char kSElementTemplate[] =
     "<S t=\"%" PRIu64 "\" d=\"%" PRIu64 "\" r=\"%" PRIu64 "\"/>\n";
 const char kSElementTemplateWithoutR[] =
@@ -55,6 +59,16 @@ void CheckIdEqual(uint32_t expected_id, T* node) {
   xml::ScopedXmlPtr<xmlNode>::type node_xml(node->GetXml());
   ASSERT_NO_FATAL_FAILURE(ExpectXmlElementIdEqual(node_xml.get(), expected_id));
 }
+
+void ExpectAttributeHasString(base::StringPiece attribute,
+                              base::StringPiece expected_value,
+                              xmlNodePtr node) {
+  xml::ScopedXmlPtr<xmlChar>::type attribute_xml_str(
+      xmlGetProp(node, BAD_CAST attribute.data()));
+  ASSERT_TRUE(attribute_xml_str);
+  EXPECT_STREQ(expected_value.data(),
+               reinterpret_cast<const char*>(attribute_xml_str.get()));
+}
 }  // namespace
 
 template <MpdBuilder::MpdType type>
@@ -95,6 +109,10 @@ class MpdBuilderTest: public ::testing::Test {
 
 class StaticMpdBuilderTest : public MpdBuilderTest<MpdBuilder::kStatic> {};
 
+// Use this test name for things that are common to both static an dynamic
+// mpd builder tests.
+typedef StaticMpdBuilderTest CommonMpdBuilderTest;
+
 class DynamicMpdBuilderTest : public MpdBuilderTest<MpdBuilder::kDynamic> {
  public:
   virtual ~DynamicMpdBuilderTest() {}
@@ -114,6 +132,9 @@ class DynamicMpdBuilderTest : public MpdBuilderTest<MpdBuilder::kDynamic> {
         "  width: 720\n"
         "  height: 480\n"
         "  time_scale: 10\n"
+        "  frame_duration: 5\n"
+        "  pixel_width: 1\n"
+        "  pixel_height: 1\n"
         "}\n"
         "reference_time_scale: %u\n"
         "container_type: 1\n"
@@ -182,7 +203,7 @@ class SegmentTemplateTest : public DynamicMpdBuilderTest {
         "    <AdaptationSet id=\"0\">\n"
         "      <Representation id=\"0\" bandwidth=\"%" PRIu64 "\" "
         "codecs=\"avc1.010101\" mimeType=\"video/mp4\" width=\"720\" "
-        "height=\"480\">\n"
+        "height=\"480\" frameRate=\"10/5\" sar=\"1:1\">\n"
         "        <SegmentTemplate timescale=\"1000\" "
         "initialization=\"init.mp4\" media=\"$Time$.mp4\">\n"
         "          <SegmentTimeline>\n%s"
@@ -233,6 +254,9 @@ class TimeShiftBufferDepthTest : public SegmentTemplateTest {
         "  width: 720\n"
         "  height: 480\n"
         "  time_scale: 10\n"
+        "  frame_duration: 2\n"
+        "  pixel_width: 1\n"
+        "  pixel_height: 1\n"
         "}\n"
         "reference_time_scale: %u\n"
         "container_type: 1\n"
@@ -261,7 +285,7 @@ class TimeShiftBufferDepthTest : public SegmentTemplateTest {
         "    <AdaptationSet id=\"0\">\n"
         "      <Representation id=\"0\" bandwidth=\"%" PRIu64 "\" "
         "codecs=\"avc1.010101\" mimeType=\"video/mp4\" width=\"720\" "
-        "height=\"480\">\n"
+        "height=\"480\" frameRate=\"10/2\" sar=\"1:1\">\n"
         "        <SegmentTemplate timescale=\"1000\" "
         "initialization=\"init.mp4\" media=\"$Number$.mp4\" "
         "startNumber=\"%d\">\n"
@@ -289,7 +313,74 @@ class TimeShiftBufferDepthTest : public SegmentTemplateTest {
   }
 };
 
-TEST_F(StaticMpdBuilderTest, CheckAdaptationSetId) {
+// Verify that Representation::Init() works with all "required" fields of
+// MedieInfo proto.
+TEST_F(CommonMpdBuilderTest, ValidMediaInfo) {
+  const char kTestMediaInfo[] =
+      "video_info {\n"
+      "  codec: \"avc1\"\n"
+      "  width: 720\n"
+      "  height: 480\n"
+      "  time_scale: 10\n"
+      "  frame_duration: 10\n"
+      "  pixel_width: 1\n"
+      "  pixel_height: 1\n"
+      "}\n"
+      "container_type: 1\n";
+  Representation representation(
+      ConvertToMediaInfo(kTestMediaInfo), MpdOptions(), kAnyRepresentationId);
+  EXPECT_TRUE(representation.Init());
+}
+
+// Verify that Representation::Init() fails if a required field is missing.
+TEST_F(CommonMpdBuilderTest, InvalidMediaInfo) {
+  // Missing width.
+  const char kTestMediaInfo[] =
+      "video_info {\n"
+      "  codec: \"avc1\"\n"
+      "  height: 480\n"
+      "  time_scale: 10\n"
+      "  frame_duration: 10\n"
+      "  pixel_width: 1\n"
+      "  pixel_height: 1\n"
+      "}\n"
+      "container_type: 1\n";
+  Representation representation(
+      ConvertToMediaInfo(kTestMediaInfo), MpdOptions(), kAnyRepresentationId);
+  EXPECT_FALSE(representation.Init());
+}
+
+// Basic check that the fields in video info are in the XML.
+TEST_F(CommonMpdBuilderTest, CheckVideoInfoReflectedInXml) {
+  const char kTestMediaInfo[] =
+      "video_info {\n"
+      "  codec: \"avc1\"\n"
+      "  width: 1280\n"
+      "  height: 720\n"
+      "  time_scale: 10\n"
+      "  frame_duration: 10\n"
+      "  pixel_width: 1\n"
+      "  pixel_height: 1\n"
+      "}\n"
+      "container_type: 1\n";
+  Representation representation(
+      ConvertToMediaInfo(kTestMediaInfo), MpdOptions(), kAnyRepresentationId);
+  EXPECT_TRUE(representation.Init());
+
+  xml::ScopedXmlPtr<xmlNode>::type node_xml(representation.GetXml());
+  EXPECT_NO_FATAL_FAILURE(
+      ExpectAttributeHasString("codecs", "avc1", node_xml.get()));
+  EXPECT_NO_FATAL_FAILURE(
+      ExpectAttributeHasString("width", "1280", node_xml.get()));
+  EXPECT_NO_FATAL_FAILURE(
+      ExpectAttributeHasString("height", "720", node_xml.get()));
+  EXPECT_NO_FATAL_FAILURE(
+      ExpectAttributeHasString("sar", "1:1", node_xml.get()));
+  EXPECT_NO_FATAL_FAILURE(
+      ExpectAttributeHasString("frameRate", "10/10", node_xml.get()));
+}
+
+TEST_F(CommonMpdBuilderTest, CheckAdaptationSetId) {
   base::AtomicSequenceNumber sequence_counter;
   const uint32_t kAdaptationSetId = 42;
 
@@ -298,7 +389,7 @@ TEST_F(StaticMpdBuilderTest, CheckAdaptationSetId) {
   ASSERT_NO_FATAL_FAILURE(CheckIdEqual(kAdaptationSetId, &adaptation_set));
 }
 
-TEST_F(StaticMpdBuilderTest, CheckRepresentationId) {
+TEST_F(CommonMpdBuilderTest, CheckRepresentationId) {
   const MediaInfo video_media_info = GetTestMediaInfo(kFileNameVideoMediaInfo1);
   const uint32_t kRepresentationId = 1;
 

packager/mpd/base/xml/xml_node.cc

@@ -330,7 +330,7 @@ bool RepresentationXmlNode::AddVideoInfo(
   // Make sure that all the widths and heights match.
   for (int i = 0; i < repeated_video_info.size(); ++i) {
     const MediaInfo_VideoInfo& video_info = repeated_video_info.Get(i);
-    if (video_info.width() <= 0 || video_info.height() <= 0)
+    if (video_info.width() == 0 || video_info.height() == 0)
       return false;
 
     if (width == 0) {
@@ -352,6 +352,15 @@ bool RepresentationXmlNode::AddVideoInfo(
   if (height != 0)
     SetIntegerAttribute("height", height);
 
+  // TODO(rkuroiwa): Because we are going ot make video_info optional instead
+  // of repeated, just using the first video_info.
+  const MediaInfo_VideoInfo& video_info = repeated_video_info.Get(0);
+  SetStringAttribute("frameRate",
+                     base::IntToString(video_info.time_scale()) + "/" +
+                         base::IntToString(video_info.frame_duration()));
+
+  SetStringAttribute("sar", base::IntToString(video_info.pixel_width()) + ":" +
+                                base::IntToString(video_info.pixel_height()));
   return true;
 }
 

packager/mpd/base/xml/xml_node.h

@@ -147,8 +147,8 @@ class RepresentationXmlNode : public RepresentationBaseXmlNode {
   ///         applicable), false otherwise.
   bool AddAudioInfo(const RepeatedAudioInfo& repeated_audio_info);
 
-  /// Adds fields that are specific to VOD. This ignores @a media_info fields for
+  /// Adds fields that are specific to VOD. This ignores @a media_info fields
-  /// Live.
+  /// for Live.
   /// @param media_info is a MediaInfo with VOD information.
   /// @return true on success, false otherwise.
   bool AddVODOnlyInfo(const MediaInfo& media_info);

packager/mpd/test/data/audio_media_info1_video_media_info1_expected_mpd_output.txt

@@ -2,7 +2,7 @@
 <MPD xmlns="urn:mpeg:DASH:schema:MPD:2011" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xlink="http://www.w3.org/1999/xlink" xsi:schemaLocation="urn:mpeg:DASH:schema:MPD:2011 DASH-MPD.xsd" minBufferTime="PT2S" type="static" profiles="urn:mpeg:dash:profile:isoff-on-demand:2011" mediaPresentationDuration="PT10.5S">
   <Period>
     <AdaptationSet id="0">
-      <Representation id="1" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480">
+      <Representation id="1" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480" frameRate="10/1" sar="1:1">
         <BaseURL>test_output_file_name1.mp4</BaseURL>
         <SegmentBase indexRange="121-221" timescale="1000">
           <Initialization range="0-120"/>

packager/mpd/test/data/dynamic_normal_mpd.txt

@@ -2,7 +2,7 @@
 <MPD xmlns="urn:mpeg:DASH:schema:MPD:2011" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xlink="http://www.w3.org/1999/xlink" xsi:schemaLocation="urn:mpeg:DASH:schema:MPD:2011 DASH-MPD.xsd" availabilityStartTime="2011-12-25T12:30:00" minBufferTime="PT2S" type="dynamic" profiles="urn:mpeg:dash:profile:isoff-live:2011">
   <Period start="PT0S">
     <AdaptationSet id="0">
-      <Representation id="0" bandwidth="102400" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480">
+      <Representation id="0" bandwidth="102400" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480" frameRate="10/5" sar="1:1">
         <SegmentTemplate timescale="1000" initialization="init.mp4" media="$Time$.mp4">
           <SegmentTimeline>
             <S t="0" d="10"/>

packager/mpd/test/data/language_audio_media_info_expected_output.txt

@@ -2,7 +2,7 @@
 <MPD xmlns="urn:mpeg:DASH:schema:MPD:2011" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xlink="http://www.w3.org/1999/xlink" xsi:schemaLocation="urn:mpeg:DASH:schema:MPD:2011 DASH-MPD.xsd" minBufferTime="PT2S" type="static" profiles="urn:mpeg:dash:profile:isoff-on-demand:2011" mediaPresentationDuration="PT10.5S">
   <Period>
     <AdaptationSet id="0">
-      <Representation id="3" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480">
+      <Representation id="3" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480" frameRate="10/1" sar="1:1">
         <BaseURL>test_output_file_name1.mp4</BaseURL>
         <SegmentBase indexRange="121-221" timescale="1000">
           <Initialization range="0-120"/>

packager/mpd/test/data/language_video_media_info1.txt

@@ -4,6 +4,9 @@ video_info {
   width: 720
   height: 480
   time_scale: 10
+  frame_duration: 1
+  pixel_width: 1
+  pixel_height: 1
 }
 init_range {
   begin: 0

packager/mpd/test/data/video_media_info1.txt

@@ -4,6 +4,9 @@ video_info {
   width: 720
   height: 480
   time_scale: 10
+  frame_duration: 1
+  pixel_width: 1
+  pixel_height: 1
 }
 init_range {
   begin: 0

packager/mpd/test/data/video_media_info1_expected_mpd_output.txt

@@ -2,7 +2,7 @@
 <MPD xmlns="urn:mpeg:DASH:schema:MPD:2011" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xlink="http://www.w3.org/1999/xlink" xsi:schemaLocation="urn:mpeg:DASH:schema:MPD:2011 DASH-MPD.xsd" minBufferTime="PT2S" type="static" profiles="urn:mpeg:dash:profile:isoff-on-demand:2011" mediaPresentationDuration="PT10.5S">
   <Period>
     <AdaptationSet id="0">
-      <Representation id="0" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480">
+      <Representation id="0" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480" frameRate="10/1" sar="1:1">
         <BaseURL>test_output_file_name1.mp4</BaseURL>
         <SegmentBase indexRange="121-221" timescale="1000">
           <Initialization range="0-120"/>

packager/mpd/test/data/video_media_info1and2_expected_mpd_output.txt

@@ -2,13 +2,13 @@
 <MPD xmlns="urn:mpeg:DASH:schema:MPD:2011" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xlink="http://www.w3.org/1999/xlink" xsi:schemaLocation="urn:mpeg:DASH:schema:MPD:2011 DASH-MPD.xsd" minBufferTime="PT2S" type="static" profiles="urn:mpeg:dash:profile:isoff-on-demand:2011" mediaPresentationDuration="PT10.5S">
   <Period>
     <AdaptationSet id="0">
-      <Representation id="0" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480">
+      <Representation id="0" bandwidth="7620" codecs="avc1.010101" mimeType="video/mp4" width="720" height="480" frameRate="10/1" sar="1:1">
         <BaseURL>test_output_file_name1.mp4</BaseURL>
         <SegmentBase indexRange="121-221" timescale="1000">
           <Initialization range="0-120"/>
         </SegmentBase>
       </Representation>
-      <Representation id="1" bandwidth="5000" codecs="avc1.010101" mimeType="video/mp4" width="480" height="360">
+      <Representation id="1" bandwidth="5000" codecs="avc1.010101" mimeType="video/mp4" width="480" height="360" frameRate="20/20" sar="2:1">
         <BaseURL>test_output_file_name2.mp4</BaseURL>
         <SegmentBase indexRange="54-100" timescale="50">
           <Initialization range="0-53"/>

packager/mpd/test/data/video_media_info2.txt

@@ -4,6 +4,9 @@ video_info {
   width: 480
   height: 360
   time_scale: 20
+  frame_duration: 20
+  pixel_width: 2
+  pixel_height: 1
 }
 init_range {
   begin: 0

packager/mpd/test/mpd_builder_test_helper.h

@@ -16,6 +16,9 @@ namespace edash_packager {
 class MediaInfo;
 
 // File names that could be used to call GetTestDataFilePath().
+// TODO(rkuroiwa): Seems like too may indirection. Maybe put the definition
+// of the proto instance in this file. Or just remove this and put it in the
+// test.
 const char kFileNameVideoMediaInfo1[] = "video_media_info1.txt";
 const char kFileNameVideoMediaInfo2[] = "video_media_info2.txt";
 const char kFileNameAudioMediaInfo1[] = "audio_media_info1.txt";