From 2756902934135de7001430d7c9f06359576b2ef3 Mon Sep 17 00:00:00 2001 From: Rintaro Kuroiwa Date: Sat, 16 Apr 2016 15:58:47 -0700 Subject: [PATCH] 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 | 4 ++-- packager/hls/base/master_playlist_unittest.cc | 10 +++++----- packager/hls/base/media_playlist.cc | 16 ++++++++++------ packager/hls/base/media_playlist.h | 3 ++- packager/hls/base/media_playlist_unittest.cc | 10 ++++++++-- packager/hls/base/simple_hls_notifier.cc | 6 ------ .../hls/base/simple_hls_notifier_unittest.cc | 10 ---------- 7 files changed, 27 insertions(+), 32 deletions(-) diff --git a/packager/hls/base/master_playlist.cc b/packager/hls/base/master_playlist.cc index af4c4e9d8e..a6e5322c09 100644 --- a/packager/hls/base/master_playlist.cc +++ b/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()); diff --git a/packager/hls/base/master_playlist_unittest.cc b/packager/hls/base/master_playlist_unittest.cc index c8f591602a..70c9a6bb35 100644 --- a/packager/hls/base/master_playlist_unittest.cc +++ b/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"; diff --git a/packager/hls/base/media_playlist.cc b/packager/hls/base/media_playlist.cc index 6347b2d370..13d150e04d 100644 --- a/packager/hls/base/media_playlist.cc +++ b/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(duration) / time_scale_; - if (segment_duration > longest_segment_duration_) - longest_segment_duration_ = segment_duration; + const double segment_duration_seconds = + static_cast(duration) / time_scale_; + 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 { diff --git a/packager/hls/base/media_playlist.h b/packager/hls/base/media_playlist.h index aaa6e58ca0..979807cb65 100644 --- a/packager/hls/base/media_playlist.h +++ b/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_; diff --git a/packager/hls/base/media_playlist_unittest.cc b/packager/hls/base/media_playlist_unittest.cc index a69c8937c1..d67790d45f 100644 --- a/packager/hls/base/media_playlist_unittest.cc +++ b/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. - EXPECT_EQ(200000u, media_playlist_.Bitrate()); + // 200KB per second which is 1600K bits / sec. + 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"; diff --git a/packager/hls/base/simple_hls_notifier.cc b/packager/hls/base/simple_hls_notifier.cc index 2ffa5ddb59..b2d11ca06c 100644 --- a/packager/hls/base/simple_hls_notifier.cc +++ b/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 media_playlist = diff --git a/packager/hls/base/simple_hls_notifier_unittest.cc b/packager/hls/base/simple_hls_notifier_unittest.cc index 3de47ab469..b6e600bb8d 100644 --- a/packager/hls/base/simple_hls_notifier_unittest.cc +++ b/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));