From ae17159b735a0ae12dcadd2420aae318931415a7 Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Fri, 6 Oct 2017 17:01:56 -0700 Subject: [PATCH] [HLS] Fix init segment URL isn't respecting folder locations Problem occurs when the media playlists are in a sub-directory under master playlist. If base_url is not set, segment URL should be relative to the media playlist. This only affects fMP4 as TS does not have init segment. Fixes #253 Change-Id: Icddd9ed500d0a705e8b3260bfd4e916ecbba3f28 --- packager/hls/base/simple_hls_notifier.cc | 10 +++- .../hls/base/simple_hls_notifier_unittest.cc | 53 +++++++++++++++++-- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/packager/hls/base/simple_hls_notifier.cc b/packager/hls/base/simple_hls_notifier.cc index bb419a3240..635d4d17c0 100644 --- a/packager/hls/base/simple_hls_notifier.cc +++ b/packager/hls/base/simple_hls_notifier.cc @@ -275,7 +275,15 @@ bool SimpleHlsNotifier::NotifyNewStream(const MediaInfo& media_info, std::unique_ptr media_playlist = media_playlist_factory_->Create(playlist_type(), time_shift_buffer_depth_, playlist_name, name, group_id); - if (!media_playlist->SetMediaInfo(media_info)) { + + // Update init_segment_name to be relative to playlist path if needed. + MediaInfo media_info_copy = media_info; + if (media_info_copy.has_init_segment_name()) { + media_info_copy.set_init_segment_name( + GenerateSegmentUrl(media_info_copy.init_segment_name(), prefix_, + output_dir_, media_playlist->file_name())); + } + if (!media_playlist->SetMediaInfo(media_info_copy)) { LOG(ERROR) << "Failed to set media info for playlist " << playlist_name; return false; } diff --git a/packager/hls/base/simple_hls_notifier_unittest.cc b/packager/hls/base/simple_hls_notifier_unittest.cc index 3b467f73a9..fd10241184 100644 --- a/packager/hls/base/simple_hls_notifier_unittest.cc +++ b/packager/hls/base/simple_hls_notifier_unittest.cc @@ -24,6 +24,7 @@ namespace hls { using ::testing::Eq; using ::testing::InSequence; using ::testing::Mock; +using ::testing::Property; using ::testing::Return; using ::testing::StrEq; using ::testing::_; @@ -156,7 +157,9 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrl) { new MockMediaPlaylist(kVodPlaylist, "playlist.m3u8", "", ""); EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); - EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true)); + EXPECT_CALL(*mock_media_playlist, + SetMediaInfo(Property(&MediaInfo::init_segment_name, StrEq("")))) + .WillOnce(Return(true)); // Verify that the common prefix is stripped for AddSegment(). EXPECT_CALL( @@ -184,6 +187,43 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrl) { kAnySize)); } +TEST_F(SimpleHlsNotifierTest, RebaseInitSegmentUrl) { + std::unique_ptr mock_master_playlist( + new MockMasterPlaylist()); + std::unique_ptr factory( + new MockMediaPlaylistFactory()); + + // Pointer released by SimpleHlsNotifier. + MockMediaPlaylist* mock_media_playlist = + new MockMediaPlaylist(kVodPlaylist, "playlist.m3u8", "", ""); + EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); + + // Verify that the common prefix is stripped in init segment. + EXPECT_CALL( + *mock_media_playlist, + SetMediaInfo(Property(&MediaInfo::init_segment_name, + StrEq("http://testprefix.com/path/to/init.mp4")))) + .WillOnce(Return(true)); + + EXPECT_CALL(*factory, CreateMock(kVodPlaylist, Eq(kTestTimeShiftBufferDepth), + StrEq("video_playlist.m3u8"), StrEq("name"), + StrEq("groupid"))) + .WillOnce(Return(mock_media_playlist)); + + SimpleHlsNotifier notifier(kVodPlaylist, kTestTimeShiftBufferDepth, + kTestPrefix, kAnyOutputDir, kMasterPlaylistName); + + InjectMasterPlaylist(std::move(mock_master_playlist), ¬ifier); + InjectMediaPlaylistFactory(std::move(factory), ¬ifier); + + EXPECT_TRUE(notifier.Init()); + MediaInfo media_info; + media_info.set_init_segment_name("anything/path/to/init.mp4"); + uint32_t stream_id; + EXPECT_TRUE(notifier.NotifyNewStream(media_info, "video_playlist.m3u8", + "name", "groupid", &stream_id)); +} + TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrlRelativeToPlaylist) { std::unique_ptr mock_master_playlist( new MockMasterPlaylist()); @@ -195,11 +235,15 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrlRelativeToPlaylist) { new MockMediaPlaylist(kVodPlaylist, "video/playlist.m3u8", "", ""); EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); - EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true)); + // Verify that the init segment URL is relative to playlist path. + EXPECT_CALL(*mock_media_playlist, + SetMediaInfo(Property(&MediaInfo::init_segment_name, + StrEq("path/to/init.mp4")))) + .WillOnce(Return(true)); // Verify that the segment URL is relative to playlist path. EXPECT_CALL(*mock_media_playlist, - AddSegment(StrEq("path/to/media1.ts"), _, _, _, _)); + AddSegment(StrEq("path/to/media1.m4s"), _, _, _, _)); EXPECT_CALL(*factory, CreateMock(kVodPlaylist, Eq(kTestTimeShiftBufferDepth), StrEq("video/playlist.m3u8"), StrEq("name"), StrEq("groupid"))) @@ -213,11 +257,12 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrlRelativeToPlaylist) { EXPECT_TRUE(notifier.Init()); MediaInfo media_info; + media_info.set_init_segment_name("anything/video/path/to/init.mp4"); uint32_t stream_id; EXPECT_TRUE(notifier.NotifyNewStream(media_info, "video/playlist.m3u8", "name", "groupid", &stream_id)); EXPECT_TRUE( - notifier.NotifyNewSegment(stream_id, "anything/video/path/to/media1.ts", + notifier.NotifyNewSegment(stream_id, "anything/video/path/to/media1.m4s", kAnyStartTime, kAnyDuration, 0, kAnySize)); }