Fix small bugs in HLS playlist generation

- Media Playlist that is non-VERSION 1 must have EXT-X-VERSION.
- Attribute for EXTINF should be CODECS instead of CODEC.
- BANDWIDTH attribute expects bitrate (not bytes per second).
- Removed unnecessary check if MediaInfo has media_info_name, in
  SimpleHlsNotifier::NotifyNewStream().

Change-Id: Ia63cfa59e5e2ec24bbf1b784164e6e41176fc589

packager/hls/base/master_playlist.cc

@@ -125,7 +125,7 @@ bool MasterPlaylist::WriteMasterPlaylist(const std::string& base_url,
       const std::string& audio_codec = audio_playlists.front()->codec();
       base::StringAppendF(
           &video_output,
-          "#EXT-X-STREAM-INF:AUDIO=\"%s\",CODEC=\"%s\",BANDWIDTH=%" PRIu64 "\n"
+          "#EXT-X-STREAM-INF:AUDIO=\"%s\",CODECS=\"%s\",BANDWIDTH=%" PRIu64 "\n"
           "%s\n",
           group_id.c_str(), (video_codec + "," + audio_codec).c_str(),
           video_bitrate + max_audio_bitrate,
@@ -138,7 +138,7 @@ bool MasterPlaylist::WriteMasterPlaylist(const std::string& base_url,
       const std::string& video_codec = video_playlist->codec();
       const uint64_t video_bitrate = video_playlist->Bitrate();
       base::StringAppendF(&video_output,
-                          "#EXT-X-STREAM-INF:CODEC=\"%s\",BANDWIDTH=%" PRIu64
+                          "#EXT-X-STREAM-INF:CODECS=\"%s\",BANDWIDTH=%" PRIu64
                           "\n%s\n",
                           video_codec.c_str(), video_bitrate,
                           (base_url + video_playlist->file_name()).c_str());

packager/hls/base/master_playlist_unittest.cc

@@ -82,7 +82,7 @@ TEST_F(MasterPlaylistTest, WriteMasterPlaylistOneVideo) {
 
   const std::string expected =
       "#EXTM3U\n"
-      "#EXT-X-STREAM-INF:CODEC=\"avc1\",BANDWIDTH=435889\n"
+      "#EXT-X-STREAM-INF:CODECS=\"avc1\",BANDWIDTH=435889\n"
       "http://myplaylistdomain.com/media1.m3u8\n";
 
   ASSERT_EQ(expected, actual);
@@ -152,11 +152,11 @@ TEST_F(MasterPlaylistTest, WriteMasterPlaylistVideoAndAudio) {
       "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"audiogroup\",NAME=\"espanol\","
       "URI=\"http://playlists.org/spa.m3u8\"\n"
       "#EXT-X-STREAM-INF:AUDIO=\"audiogroup\","
-      "CODEC=\"sdvideocodec,audiocodec\","
+      "CODECS=\"sdvideocodec,audiocodec\","
       "BANDWIDTH=360000\n"
       "http://playlists.org/sd.m3u8\n"
       "#EXT-X-STREAM-INF:AUDIO=\"audiogroup\","
-      "CODEC=\"hdvideocodec,audiocodec\","
+      "CODECS=\"hdvideocodec,audiocodec\","
       "BANDWIDTH=760000\n"
       "http://playlists.org/hd.m3u8\n";
 
@@ -214,11 +214,11 @@ TEST_F(MasterPlaylistTest, WriteMasterPlaylistMultipleAudioGroups) {
       "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"audio_lo\",NAME=\"english_lo\","
       "URI=\"http://anydomain.com/eng_lo.m3u8\"\n"
       "#EXT-X-STREAM-INF:AUDIO=\"audio_hi\","
-      "CODEC=\"videocodec,audiocodec_hi\","
+      "CODECS=\"videocodec,audiocodec_hi\","
       "BANDWIDTH=400000\n"
       "http://anydomain.com/video.m3u8\n"
       "#EXT-X-STREAM-INF:AUDIO=\"audio_lo\","
-      "CODEC=\"videocodec,audiocodec_lo\","
+      "CODECS=\"videocodec,audiocodec_lo\","
       "BANDWIDTH=350000\n"
       "http://anydomain.com/video.m3u8\n";
 

packager/hls/base/media_playlist.cc

@@ -169,15 +169,16 @@ void MediaPlaylist::AddSegment(const std::string& file_name,
     return;
   }
 
-  const double segment_duration = static_cast<double>(duration) / time_scale_;
+  const double segment_duration_seconds =
-  if (segment_duration > longest_segment_duration_)
+      static_cast<double>(duration) / time_scale_;
-    longest_segment_duration_ = segment_duration;
+  if (segment_duration_seconds > longest_segment_duration_)
+    longest_segment_duration_ = segment_duration_seconds;
 
-  total_duration_in_seconds_ += segment_duration;
+  total_duration_in_seconds_ += segment_duration_seconds;
   total_segments_size_ += size;
   ++total_num_segments_;
 
-  entries_.push_back(new SegmentInfoEntry(file_name, segment_duration));
+  entries_.push_back(new SegmentInfoEntry(file_name, segment_duration_seconds));
 }
 
 // TODO(rkuroiwa): This works for single key format but won't work for multiple
@@ -264,7 +265,9 @@ bool MediaPlaylist::WriteToFile(media::File* file) {
     SetTargetDuration(ceil(GetLongestSegmentDuration()));
   }
 
+  // EXTINF with floating point duration requires version 4.
   std::string header = base::StringPrintf("#EXTM3U\n"
+                                          "#EXT-X-VERSION:4\n"
                                           "#EXT-X-TARGETDURATION:%d\n",
                                           target_duration_);
   std::string body;
@@ -298,7 +301,8 @@ uint64_t MediaPlaylist::Bitrate() const {
     return 0;
   if (total_segments_size_ == 0)
     return 0;
-  return total_segments_size_ / total_duration_in_seconds_;
+  const int kBytesToBits = 8;
+  return total_segments_size_ * kBytesToBits / total_duration_in_seconds_;
 }
 
 double MediaPlaylist::GetLongestSegmentDuration() const {

packager/hls/base/media_playlist.h

@@ -128,7 +128,7 @@ class MediaPlaylist {
   /// If bitrate is specified in MediaInfo then it will use that value.
   /// Otherwise, it is calculated from the duration and the size of the
   /// segments added to this object.
-  /// @return the bitrate of this MediaPlaylist.
+  /// @return the bitrate (in bits per second) of this MediaPlaylist.
   virtual uint64_t Bitrate() const;
 
   /// @return the longest segment’s duration. This will return 0 if no
@@ -157,6 +157,7 @@ class MediaPlaylist {
   double longest_segment_duration_ = 0.0;
   uint32_t time_scale_ = 0;
 
+  // The sum of the size of the segments listed in this playlist (in bytes).
   uint64_t total_segments_size_ = 0;
   double total_duration_in_seconds_ = 0.0;
   int total_num_segments_;

packager/hls/base/media_playlist_unittest.cc

@@ -113,6 +113,7 @@ TEST_F(MediaPlaylistTest, WriteToFile) {
   ASSERT_TRUE(media_playlist_.SetMediaInfo(valid_video_media_info_));
   const std::string kExpectedOutput =
       "#EXTM3U\n"
+      "#EXT-X-VERSION:4\n"
       "#EXT-X-TARGETDURATION:0\n";
 
   MockFile file;
@@ -142,8 +143,8 @@ TEST_F(MediaPlaylistTest, GetBitrateFromSegments) {
   // 20 seconds, 5MB.
   media_playlist_.AddSegment("file2.ts", 1800000, 5000000);
 
-  // 200KB per second.
+  // 200KB per second which is 1600K bits / sec.
-  EXPECT_EQ(200000u, media_playlist_.Bitrate());
+  EXPECT_EQ(1600000u, media_playlist_.Bitrate());
 }
 
 TEST_F(MediaPlaylistTest, GetLongestSegmentDuration) {
@@ -165,6 +166,7 @@ TEST_F(MediaPlaylistTest, SetTargetDuration) {
   EXPECT_TRUE(media_playlist_.SetTargetDuration(20));
   const std::string kExpectedOutput =
       "#EXTM3U\n"
+      "#EXT-X-VERSION:4\n"
       "#EXT-X-TARGETDURATION:20\n";
 
   MockFile file;
@@ -188,6 +190,7 @@ TEST_F(MediaPlaylistTest, WriteToFileWithSegments) {
   media_playlist_.AddSegment("file2.ts", 2700000, 5000000);
   const std::string kExpectedOutput =
       "#EXTM3U\n"
+      "#EXT-X-VERSION:4\n"
       "#EXT-X-TARGETDURATION:30\n"
       "#EXTINF:10.000\n"
       "file1.ts\n"
@@ -214,6 +217,7 @@ TEST_F(MediaPlaylistTest, WriteToFileWithEncryptionInfo) {
   media_playlist_.AddSegment("file2.ts", 2700000, 5000000);
   const std::string kExpectedOutput =
       "#EXTM3U\n"
+      "#EXT-X-VERSION:4\n"
       "#EXT-X-TARGETDURATION:30\n"
       "#EXT-X-KEY:METHOD=SAMPLE-AES,"
       "URI=\"http://example.com\",IV=0x12345678,KEYFORMATVERSIONS=\"1/2/4\","
@@ -243,6 +247,7 @@ TEST_F(MediaPlaylistTest, WriteToFileWithEncryptionInfoEmptyIv) {
   media_playlist_.AddSegment("file2.ts", 2700000, 5000000);
   const std::string kExpectedOutput =
       "#EXTM3U\n"
+      "#EXT-X-VERSION:4\n"
       "#EXT-X-TARGETDURATION:30\n"
       "#EXT-X-KEY:METHOD=SAMPLE-AES,"
       "URI=\"http://example.com\",KEYFORMAT=\"com.widevine\"\n"
@@ -270,6 +275,7 @@ TEST_F(MediaPlaylistTest, RemoveOldestSegment) {
 
   const std::string kExpectedOutput =
       "#EXTM3U\n"
+      "#EXT-X-VERSION:4\n"
       "#EXT-X-TARGETDURATION:30\n"
       "#EXTINF:30.000\n"
       "file2.ts\n";

packager/hls/base/simple_hls_notifier.cc

@@ -58,12 +58,6 @@ bool SimpleHlsNotifier::NotifyNewStream(const MediaInfo& media_info,
                                         const std::string& group_id,
                                         uint32_t* stream_id) {
   DCHECK(stream_id);
-  if (!media_info.has_media_file_name()) {
-    LOG(ERROR) << "MediaInfo.media_file_name is required to generate a Media "
-                  "Playlist name.";
-    return false;
-  }
-
   *stream_id = sequence_number_.GetNext();
 
   scoped_ptr<MediaPlaylist> media_playlist =

packager/hls/base/simple_hls_notifier_unittest.cc

@@ -93,8 +93,6 @@ TEST_F(SimpleHlsNotifierTest, NotifyNewStream) {
   InjectMediaPlaylistFactory(factory.Pass());
   EXPECT_TRUE(notifier_.Init());
   MediaInfo media_info;
-  media_info.set_media_file_name("media_file.mp4");
-  media_info.mutable_video_info()->set_codec("videocodec");
   uint32_t stream_id;
   EXPECT_TRUE(notifier_.NotifyNewStream(media_info, "video_playlist.m3u8",
                                         "name", "groupid", &stream_id));
@@ -126,8 +124,6 @@ TEST_F(SimpleHlsNotifierTest, NotifyNewSegment) {
   InjectMediaPlaylistFactory(factory.Pass());
   EXPECT_TRUE(notifier_.Init());
   MediaInfo media_info;
-  media_info.set_media_file_name("media_file.mp4");
-  media_info.mutable_video_info()->set_codec("videocodec");
   uint32_t stream_id;
   EXPECT_TRUE(notifier_.NotifyNewStream(media_info, "playlist.m3u8", "name",
                                         "groupid", &stream_id));
@@ -159,8 +155,6 @@ TEST_F(SimpleHlsNotifierTest, NotifyEncryptionUpdate) {
   InjectMediaPlaylistFactory(factory.Pass());
   EXPECT_TRUE(notifier_.Init());
   MediaInfo media_info;
-  media_info.set_media_file_name("media_file.mp4");
-  media_info.mutable_video_info()->set_codec("videocodec");
   uint32_t stream_id;
   EXPECT_TRUE(notifier_.NotifyNewStream(media_info, "playlist.m3u8", "name",
                                         "groupid", &stream_id));
@@ -225,8 +219,6 @@ TEST_F(SimpleHlsNotifierTest, MultipleKeyIdsInPssh) {
   InjectMediaPlaylistFactory(factory.Pass());
   EXPECT_TRUE(notifier_.Init());
   MediaInfo media_info;
-  media_info.set_media_file_name("media_file.mp4");
-  media_info.mutable_video_info()->set_codec("videocodec");
   uint32_t stream_id;
   EXPECT_TRUE(notifier_.NotifyNewStream(media_info, "playlist.m3u8", "name",
                                         "groupid", &stream_id));
@@ -297,8 +289,6 @@ TEST_F(SimpleHlsNotifierTest, NotifyEncryptionUpdateEmptyIv) {
   InjectMediaPlaylistFactory(factory.Pass());
   EXPECT_TRUE(notifier_.Init());
   MediaInfo media_info;
-  media_info.set_media_file_name("media_file.mp4");
-  media_info.mutable_video_info()->set_codec("videocodec");
   uint32_t stream_id;
   EXPECT_TRUE(notifier_.NotifyNewStream(media_info, "playlist.m3u8", "name",
                                         "groupid", &stream_id));