From bc903d2d83a69d9c1f7a7e1dc02270d85aed4495 Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Fri, 8 Sep 2017 10:52:48 -0700 Subject: [PATCH] [HLS] Fix 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, media segment URL should be relative to the media playlist instead of the master playlist. Also make sure the path separator is "/" in URL instead of "\" on Windows. Fixes #253 Change-Id: I8e933750276435d94dd01bfa53ee2bc050dfd193 --- packager/hls/base/simple_hls_notifier.cc | 87 ++++++++++--------- .../hls/base/simple_hls_notifier_unittest.cc | 69 ++++++++++----- packager/mpd/base/mpd_builder.cc | 17 ++-- packager/mpd/base/mpd_builder_unittest.cc | 49 +++-------- 4 files changed, 116 insertions(+), 106 deletions(-) diff --git a/packager/hls/base/simple_hls_notifier.cc b/packager/hls/base/simple_hls_notifier.cc index eb49a116ed..bb419a3240 100644 --- a/packager/hls/base/simple_hls_notifier.cc +++ b/packager/hls/base/simple_hls_notifier.cc @@ -22,6 +22,9 @@ #include "packager/media/base/widevine_pssh_data.pb.h" namespace shaka { + +using base::FilePath; + namespace hls { namespace { @@ -59,40 +62,44 @@ std::string VectorToString(const std::vector& v) { } // TODO(rkuroiwa): Dedup these with the functions in MpdBuilder. -std::string MakePathRelative(const std::string& original_path, - const std::string& output_dir) { - return (original_path.find(output_dir) == 0) - ? original_path.substr(output_dir.size()) - : original_path; +// If |media_path| is contained in |parent_path|, then +// Strips the common path and keep only the relative part of |media_path|. +// e.g. if |parent_path| is /some/parent/ and +// |media_path| is /some/parent/abc/child/item.ext, +// abc/child/item.ext is returned. +// else +// Returns |media_path|. +// The path separator of the output is also changed to "/" if it is not. +std::string MakePathRelative(const std::string& media_path, + const FilePath& parent_path) { + FilePath relative_path; + const FilePath child_path = FilePath::FromUTF8Unsafe(media_path); + const bool is_child = + parent_path.AppendRelativePath(child_path, &relative_path); + if (!is_child) + relative_path = child_path; + return relative_path.NormalizePathSeparatorsTo('/').AsUTF8Unsafe(); } -void MakePathsRelativeToOutputDirectory(const std::string& output_dir, - MediaInfo* media_info) { - DCHECK(media_info); - const std::string kFileProtocol("file://"); - std::string prefix_stripped_output_dir = - (output_dir.find(kFileProtocol) == 0) - ? output_dir.substr(kFileProtocol.size()) - : output_dir; - - if (prefix_stripped_output_dir.empty()) - return; - - std::string directory_with_separator( - base::FilePath::FromUTF8Unsafe(prefix_stripped_output_dir) - .AsEndingWithSeparator() - .AsUTF8Unsafe()); - if (directory_with_separator.empty()) - return; - - if (media_info->has_media_file_name()) { - media_info->set_media_file_name(MakePathRelative( - media_info->media_file_name(), directory_with_separator)); - } - if (media_info->has_segment_template()) { - media_info->set_segment_template(MakePathRelative( - media_info->segment_template(), directory_with_separator)); +// Segment URL is relative to either output directory or the directory +// containing the media playlist depends on whether base_url is set. +std::string GenerateSegmentUrl(const std::string& segment_name, + const std::string& base_url, + const std::string& output_dir, + const std::string& playlist_file_name) { + FilePath output_path = FilePath::FromUTF8Unsafe(output_dir); + if (!base_url.empty()) { + // Media segment URL is base_url + segment path relative to output + // directory. + return base_url + MakePathRelative(segment_name, output_path); } + // Media segment URL is segment path relative to the directory containing the + // playlist. + const FilePath playlist_dir = + output_path.Append(FilePath::FromUTF8Unsafe(playlist_file_name)) + .DirName() + .AsEndingWithSeparator(); + return MakePathRelative(segment_name, playlist_dir); } bool WidevinePsshToJson(const std::vector& pssh_box, @@ -216,8 +223,8 @@ bool HandleWidevineKeyFormats( bool WriteMediaPlaylist(const std::string& output_dir, MediaPlaylist* playlist) { std::string file_path = - base::FilePath::FromUTF8Unsafe(output_dir) - .Append(base::FilePath::FromUTF8Unsafe(playlist->file_name())) + FilePath::FromUTF8Unsafe(output_dir) + .Append(FilePath::FromUTF8Unsafe(playlist->file_name())) .AsUTF8Unsafe(); if (!playlist->WriteToFile(file_path)) { LOG(ERROR) << "Failed to write playlist " << file_path; @@ -265,13 +272,10 @@ bool SimpleHlsNotifier::NotifyNewStream(const MediaInfo& media_info, uint32_t* stream_id) { DCHECK(stream_id); - MediaInfo adjusted_media_info(media_info); - MakePathsRelativeToOutputDirectory(output_dir_, &adjusted_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(adjusted_media_info)) { + if (!media_playlist->SetMediaInfo(media_info)) { LOG(ERROR) << "Failed to set media info for playlist " << playlist_name; return false; } @@ -311,12 +315,11 @@ bool SimpleHlsNotifier::NotifyNewSegment(uint32_t stream_id, LOG(ERROR) << "Cannot find stream with ID: " << stream_id; return false; } - const std::string relative_segment_name = - MakePathRelative(segment_name, output_dir_); - auto& media_playlist = stream_iterator->second->media_playlist; - media_playlist->AddSegment(prefix_ + relative_segment_name, start_time, - duration, start_byte_offset, size); + const std::string& segment_url = GenerateSegmentUrl( + segment_name, prefix_, output_dir_, media_playlist->file_name()); + media_playlist->AddSegment(segment_url, start_time, duration, + start_byte_offset, size); // Update target duration. uint32_t longest_segment_duration = diff --git a/packager/hls/base/simple_hls_notifier_unittest.cc b/packager/hls/base/simple_hls_notifier_unittest.cc index 70eca844c1..3b467f73a9 100644 --- a/packager/hls/base/simple_hls_notifier_unittest.cc +++ b/packager/hls/base/simple_hls_notifier_unittest.cc @@ -63,17 +63,13 @@ class MockMediaPlaylistFactory : public MediaPlaylistFactory { const double kTestTimeShiftBufferDepth = 1800.0; const char kTestPrefix[] = "http://testprefix.com/"; +const char kEmptyPrefix[] = ""; const char kAnyOutputDir[] = "anything/"; const uint64_t kAnyStartTime = 10; const uint64_t kAnyDuration = 1000; const uint64_t kAnySize = 2000; -MATCHER_P(SegmentTemplateEq, expected_template, "") { - *result_listener << " which is " << arg.segment_template(); - return arg.segment_template() == expected_template; -} - const char kCencProtectionScheme[] = "cenc"; const char kSampleAesProtectionScheme[] = "cbca"; @@ -149,7 +145,7 @@ TEST_F(SimpleHlsNotifierTest, Init) { // Verify that relative paths can be handled. // For this test, since the prefix "anything/" matches, the prefix should be // stripped. -TEST_F(SimpleHlsNotifierTest, RebaseSegmentTemplateRelative) { +TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrl) { std::unique_ptr mock_master_playlist( new MockMasterPlaylist()); std::unique_ptr factory( @@ -160,9 +156,7 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentTemplateRelative) { new MockMediaPlaylist(kVodPlaylist, "playlist.m3u8", "", ""); EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); - EXPECT_CALL(*mock_media_playlist, - SetMediaInfo(SegmentTemplateEq("path/to/media$Number$.ts"))) - .WillOnce(Return(true)); + EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true)); // Verify that the common prefix is stripped for AddSegment(). EXPECT_CALL( @@ -181,7 +175,6 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentTemplateRelative) { EXPECT_TRUE(notifier.Init()); MediaInfo media_info; - media_info.set_segment_template("anything/path/to/media$Number$.ts"); uint32_t stream_id; EXPECT_TRUE(notifier.NotifyNewStream(media_info, "video_playlist.m3u8", "name", "groupid", &stream_id)); @@ -191,10 +184,46 @@ TEST_F(SimpleHlsNotifierTest, RebaseSegmentTemplateRelative) { kAnySize)); } -// Verify that when segment template's prefix and output dir match, then the -// prefix is stripped from segment template. -TEST_F(SimpleHlsNotifierTest, - RebaseAbsoluteSegmentTemplatePrefixAndOutputDirMatch) { +TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrlRelativeToPlaylist) { + 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, "video/playlist.m3u8", "", ""); + EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); + + EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true)); + + // Verify that the segment URL is relative to playlist path. + EXPECT_CALL(*mock_media_playlist, + AddSegment(StrEq("path/to/media1.ts"), _, _, _, _)); + EXPECT_CALL(*factory, CreateMock(kVodPlaylist, Eq(kTestTimeShiftBufferDepth), + StrEq("video/playlist.m3u8"), StrEq("name"), + StrEq("groupid"))) + .WillOnce(Return(mock_media_playlist)); + + SimpleHlsNotifier notifier(kVodPlaylist, kTestTimeShiftBufferDepth, + kEmptyPrefix, kAnyOutputDir, kMasterPlaylistName); + + InjectMasterPlaylist(std::move(mock_master_playlist), ¬ifier); + InjectMediaPlaylistFactory(std::move(factory), ¬ifier); + + EXPECT_TRUE(notifier.Init()); + MediaInfo media_info; + 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", + kAnyStartTime, kAnyDuration, 0, kAnySize)); +} + +// Verify that when segment path's prefix and output dir match, then the +// prefix is stripped from segment path. +TEST_F(SimpleHlsNotifierTest, RebaseAbsoluteSegmentPrefixAndOutputDirMatch) { const char kAbsoluteOutputDir[] = "/tmp/something/"; SimpleHlsNotifier test_notifier(kVodPlaylist, kTestTimeShiftBufferDepth, kTestPrefix, kAbsoluteOutputDir, @@ -210,9 +239,7 @@ TEST_F(SimpleHlsNotifierTest, new MockMediaPlaylist(kVodPlaylist, "playlist.m3u8", "", ""); EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); - EXPECT_CALL(*mock_media_playlist, - SetMediaInfo(SegmentTemplateEq("media$Number$.ts"))) - .WillOnce(Return(true)); + EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true)); // Verify that the output_dir is stripped and then kTestPrefix is prepended. EXPECT_CALL(*mock_media_playlist, @@ -226,7 +253,6 @@ TEST_F(SimpleHlsNotifierTest, InjectMediaPlaylistFactory(std::move(factory), &test_notifier); EXPECT_TRUE(test_notifier.Init()); MediaInfo media_info; - media_info.set_segment_template("/tmp/something/media$Number$.ts"); uint32_t stream_id; EXPECT_TRUE(test_notifier.NotifyNewStream(media_info, "video_playlist.m3u8", "name", "groupid", &stream_id)); @@ -239,7 +265,7 @@ TEST_F(SimpleHlsNotifierTest, // If the paths don't match at all and they are both absolute and completely // different, then keep it as is. TEST_F(SimpleHlsNotifierTest, - RebaseAbsoluteSegmentTemplateCompletelyDifferentDirectory) { + RebaseAbsoluteSegmentCompletelyDifferentDirectory) { const char kAbsoluteOutputDir[] = "/tmp/something/"; SimpleHlsNotifier test_notifier(kVodPlaylist, kTestTimeShiftBufferDepth, kTestPrefix, kAbsoluteOutputDir, @@ -255,10 +281,7 @@ TEST_F(SimpleHlsNotifierTest, new MockMediaPlaylist(kVodPlaylist, "playlist.m3u8", "", ""); EXPECT_CALL(*mock_master_playlist, AddMediaPlaylist(mock_media_playlist)); - EXPECT_CALL( - *mock_media_playlist, - SetMediaInfo(SegmentTemplateEq("/var/somewhereelse/media$Number$.ts"))) - .WillOnce(Return(true)); + EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true)); EXPECT_CALL(*mock_media_playlist, AddSegment("http://testprefix.com//var/somewhereelse/media1.ts", _, _, _, _)); diff --git a/packager/mpd/base/mpd_builder.cc b/packager/mpd/base/mpd_builder.cc index 684e577afe..6ea0d3bcf5 100644 --- a/packager/mpd/base/mpd_builder.cc +++ b/packager/mpd/base/mpd_builder.cc @@ -186,9 +186,15 @@ int SearchTimedOutRepeatIndex(uint64_t timeshift_limit, return (timeshift_limit - segment_info.start_time) / segment_info.duration; } -std::string MakePathRelative(const std::string& path, - const std::string& mpd_dir) { - return (path.find(mpd_dir) == 0) ? path.substr(mpd_dir.size()) : path; +std::string MakePathRelative(const std::string& media_path, + const FilePath& parent_path) { + FilePath relative_path; + const FilePath child_path = FilePath::FromUTF8Unsafe(media_path); + const bool is_child = + parent_path.AppendRelativePath(child_path, &relative_path); + if (!is_child) + relative_path = child_path; + return relative_path.NormalizePathSeparatorsTo('/').AsUTF8Unsafe(); } // Check whether the video info has width and height. @@ -627,8 +633,9 @@ void MpdBuilder::MakePathsRelativeToMpd(const std::string& mpd_path, : mpd_path; if (!mpd_file_path.empty()) { - std::string mpd_dir(FilePath::FromUTF8Unsafe(mpd_file_path) - .DirName().AsEndingWithSeparator().AsUTF8Unsafe()); + const FilePath mpd_dir(FilePath::FromUTF8Unsafe(mpd_file_path) + .DirName() + .AsEndingWithSeparator()); if (!mpd_dir.empty()) { if (media_info->has_media_file_name()) { media_info->set_media_file_name( diff --git a/packager/mpd/base/mpd_builder_unittest.cc b/packager/mpd/base/mpd_builder_unittest.cc index 67a4ddd4f0..6f6e10b1dc 100644 --- a/packager/mpd/base/mpd_builder_unittest.cc +++ b/packager/mpd/base/mpd_builder_unittest.cc @@ -2482,59 +2482,36 @@ TEST_F(TimeShiftBufferDepthTest, ManySegments) { kDefaultStartNumber + kExpectedRemovedSegments)); } +namespace { +const char kMediaFile[] = "foo/bar/media.mp4"; +const char kMediaFileBase[] = "media.mp4"; +const char kInitSegment[] = "foo/bar/init.mp4"; +const char kInitSegmentBase[] = "init.mp4"; +const char kSegmentTemplate[] = "foo/bar/segment-$Number$.mp4"; +const char kSegmentTemplateBase[] = "segment-$Number$.mp4"; +const char kPathModifiedMpd[] = "foo/bar/media.mpd"; +const char kPathNotModifiedMpd[] = "foo/baz/media.mpd"; +} // namespace + TEST(RelativePaths, PathsModified) { - const FilePath kCommonPath( - FilePath::FromUTF8Unsafe("foo").Append(FilePath::FromUTF8Unsafe("bar"))); - const std::string kMediaFileBase("media.mp4"); - const std::string kInitSegmentBase("init.mp4"); - const std::string kSegmentTemplateBase("segment-$Number$.mp4"); - const std::string kMediaFile( - kCommonPath.Append(FilePath::FromUTF8Unsafe(kMediaFileBase)) - .AsUTF8Unsafe()); - const std::string kInitSegment( - kCommonPath.Append(FilePath::FromUTF8Unsafe(kInitSegmentBase)) - .AsUTF8Unsafe()); - const std::string kSegmentTemplate( - kCommonPath.Append(FilePath::FromUTF8Unsafe(kSegmentTemplateBase)) - .AsUTF8Unsafe()); - const std::string kMpd( - kCommonPath.Append(FilePath::FromUTF8Unsafe("media.mpd")).AsUTF8Unsafe()); MediaInfo media_info; media_info.set_media_file_name(kMediaFile); media_info.set_init_segment_name(kInitSegment); media_info.set_segment_template(kSegmentTemplate); - MpdBuilder::MakePathsRelativeToMpd(kMpd, &media_info); + MpdBuilder::MakePathsRelativeToMpd(kPathModifiedMpd, &media_info); EXPECT_EQ(kMediaFileBase, media_info.media_file_name()); EXPECT_EQ(kInitSegmentBase, media_info.init_segment_name()); EXPECT_EQ(kSegmentTemplateBase, media_info.segment_template()); } TEST(RelativePaths, PathsNotModified) { - const FilePath kMediaCommon( - FilePath::FromUTF8Unsafe("foo").Append(FilePath::FromUTF8Unsafe("bar"))); - const std::string kMediaFileBase("media.mp4"); - const std::string kInitSegmentBase("init.mp4"); - const std::string kSegmentTemplateBase("segment-$Number$.mp4"); - const std::string kMediaFile( - kMediaCommon.Append(FilePath::FromUTF8Unsafe(kMediaFileBase)) - .AsUTF8Unsafe()); - const std::string kInitSegment( - kMediaCommon.Append(FilePath::FromUTF8Unsafe(kInitSegmentBase)) - .AsUTF8Unsafe()); - const std::string kSegmentTemplate( - kMediaCommon.Append(FilePath::FromUTF8Unsafe(kSegmentTemplateBase)) - .AsUTF8Unsafe()); - const std::string kMpd(FilePath::FromUTF8Unsafe("foo") - .Append(FilePath::FromUTF8Unsafe("baz")) - .Append(FilePath::FromUTF8Unsafe("media.mpd")) - .AsUTF8Unsafe()); MediaInfo media_info; media_info.set_media_file_name(kMediaFile); media_info.set_init_segment_name(kInitSegment); media_info.set_segment_template(kSegmentTemplate); - MpdBuilder::MakePathsRelativeToMpd(kMpd, &media_info); + MpdBuilder::MakePathsRelativeToMpd(kPathNotModifiedMpd, &media_info); EXPECT_EQ(kMediaFile, media_info.media_file_name()); EXPECT_EQ(kInitSegment, media_info.init_segment_name()); EXPECT_EQ(kSegmentTemplate, media_info.segment_template());