[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

@@ -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<uint8_t>& v) {
 }
 
 // TODO(rkuroiwa): Dedup these with the functions in MpdBuilder.
-std::string MakePathRelative(const std::string& original_path,
+// If |media_path| is contained in |parent_path|, then
-                             const std::string& output_dir) {
+//   Strips the common path and keep only the relative part of |media_path|.
-  return (original_path.find(output_dir) == 0)
+//   e.g. if |parent_path| is /some/parent/ and
-             ? original_path.substr(output_dir.size())
+//           |media_path| is /some/parent/abc/child/item.ext,
-             : original_path;
+//        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,
+// Segment URL is relative to either output directory or the directory
-                                        MediaInfo* media_info) {
+// containing the media playlist depends on whether base_url is set.
-  DCHECK(media_info);
+std::string GenerateSegmentUrl(const std::string& segment_name,
-  const std::string kFileProtocol("file://");
+                               const std::string& base_url,
-  std::string prefix_stripped_output_dir =
+                               const std::string& output_dir,
-      (output_dir.find(kFileProtocol) == 0)
+                               const std::string& playlist_file_name) {
-          ? output_dir.substr(kFileProtocol.size())
+  FilePath output_path = FilePath::FromUTF8Unsafe(output_dir);
-          : output_dir;
+  if (!base_url.empty()) {
-
+    // Media segment URL is base_url + segment path relative to output
-  if (prefix_stripped_output_dir.empty())
+    // directory.
-    return;
+    return base_url + MakePathRelative(segment_name, output_path);
-
-  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));
   }
+  // 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<uint8_t>& 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)
+      FilePath::FromUTF8Unsafe(output_dir)
-          .Append(base::FilePath::FromUTF8Unsafe(playlist->file_name()))
+          .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<MediaPlaylist> 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,
+  const std::string& segment_url = GenerateSegmentUrl(
-                             duration, start_byte_offset, size);
+      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 =

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<MockMasterPlaylist> mock_master_playlist(
       new MockMasterPlaylist());
   std::unique_ptr<MockMediaPlaylistFactory> 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,
+  EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true));
-              SetMediaInfo(SegmentTemplateEq("path/to/media$Number$.ts")))
-      .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
+TEST_F(SimpleHlsNotifierTest, RebaseSegmentUrlRelativeToPlaylist) {
-// prefix is stripped from segment template.
+  std::unique_ptr<MockMasterPlaylist> mock_master_playlist(
-TEST_F(SimpleHlsNotifierTest,
+      new MockMasterPlaylist());
-       RebaseAbsoluteSegmentTemplatePrefixAndOutputDirMatch) {
+  std::unique_ptr<MockMediaPlaylistFactory> 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), &notifier);
+  InjectMediaPlaylistFactory(std::move(factory), &notifier);
+
+  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,
+  EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true));
-              SetMediaInfo(SegmentTemplateEq("media$Number$.ts")))
-      .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(
+  EXPECT_CALL(*mock_media_playlist, SetMediaInfo(_)).WillOnce(Return(true));
-      *mock_media_playlist,
-      SetMediaInfo(SegmentTemplateEq("/var/somewhereelse/media$Number$.ts")))
-      .WillOnce(Return(true));
   EXPECT_CALL(*mock_media_playlist,
               AddSegment("http://testprefix.com//var/somewhereelse/media1.ts",
                          _, _, _, _));

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,
+std::string MakePathRelative(const std::string& media_path,
-                             const std::string& mpd_dir) {
+                             const FilePath& parent_path) {
-  return (path.find(mpd_dir) == 0) ? path.substr(mpd_dir.size()) : 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)
+    const FilePath mpd_dir(FilePath::FromUTF8Unsafe(mpd_file_path)
-      .DirName().AsEndingWithSeparator().AsUTF8Unsafe());
+                               .DirName()
+                               .AsEndingWithSeparator());
     if (!mpd_dir.empty()) {
       if (media_info->has_media_file_name()) {
         media_info->set_media_file_name(

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());