[HLSTree] Fix no audio due to orphan rendition with shared uri

src/parser/HLSTree.cpp

@@ -23,6 +23,7 @@
 #include <algorithm>
 #include <optional>
 #include <sstream>
+#include <unordered_set>
 
 using namespace PLAYLIST;
 using namespace UTILS;
@@ -1461,19 +1462,6 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
       rend.m_isForced = attribs["FORCED"] == "YES";
       rend.m_characteristics = attribs["CHARACTERISTICS"];
       rend.m_uri = attribs["URI"];
-      std::string uri = attribs["URI"];
-
-      if (!uri.empty())
-      {
-        // Check if this uri has been already added
-        if (std::any_of(pl.m_audioRenditions.cbegin(), pl.m_audioRenditions.cend(),
-                        [&uri](const Rendition& v) { return v.m_uri == uri; }) ||
-            std::any_of(pl.m_subtitleRenditions.cbegin(), pl.m_subtitleRenditions.cend(),
-                        [&uri](const Rendition& v) { return v.m_uri == uri; }))
-        {
-          rend.m_isUriDuplicate = true;
-        }
-      }
 
       if (streamType == StreamType::AUDIO)
         pl.m_audioRenditions.emplace_back(rend);
@@ -1518,13 +1506,6 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
       var.m_videoRange = attribs["VIDEO-RANGE"];
       var.m_uri = uri;
 
-      // Check if this uri has been already added
-      if (std::any_of(pl.m_variants.cbegin(), pl.m_variants.cend(),
-                      [&uri](const Variant& v) { return v.m_uri == uri; }))
-      {
-        var.m_isUriDuplicate = true;
-      }
-
       pl.m_variants.emplace_back(var);
     }
   }
@@ -1537,11 +1518,31 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
   period->SetStart(0);
   period->SetTimescale(TIMESCALE);
 
-  // Add audio renditions (do not take in account variants references)
+  for (Rendition& r : pl.m_audioRenditions)
+  {
+    r.m_variant = FindVariantByAudioGroupId(r.m_groupId, pl.m_variants);
+  }
+  // Sort to give priority to those have a referenced variant
+  std::stable_sort(pl.m_audioRenditions.begin(), pl.m_audioRenditions.end(),
+                   [](const Rendition& a, const Rendition& b)
+                   { return a.m_variant != nullptr && b.m_variant == nullptr; });
+
+  for (Rendition& r : pl.m_subtitleRenditions)
+  {
+    r.m_variant = FindVariantBySubtitleGroupId(r.m_groupId, pl.m_variants);
+  }
+  // Sort to give priority to those have a referenced variant
+  std::stable_sort(pl.m_subtitleRenditions.begin(), pl.m_subtitleRenditions.end(),
+                   [](const Rendition& a, const Rendition& b)
+                   { return a.m_variant != nullptr && b.m_variant == nullptr; });
+
+  std::unordered_set<std::string> collectedUris;
+
+  // Add audio renditions
   for (const Rendition& r : pl.m_audioRenditions)
   {
     // There may be multiple renditions with the same uri but different GROUP-ID
-    if (r.m_isUriDuplicate)
+    if (collectedUris.contains(r.m_uri))
       continue;
 
     auto newAdpSet = CAdaptationSet::MakeUniquePtr(period.get());
@@ -1550,11 +1551,9 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
     if (!ParseRenditon(r, newAdpSet, newRepr))
       continue;
 
-    // Find the codec string from a variant that references it
-    const Variant* varFound = FindVariantByAudioGroupId(r.m_groupId, pl.m_variants);
     std::string codecStr;
-    if (varFound)
-      codecStr = GetAudioCodec(varFound->m_codecs);
+    if (r.m_variant)
+      codecStr = GetAudioCodec(r.m_variant->m_codecs);
     else
     {
       if (manifestCfg.hlsAddOrphansRenditions)
@@ -1569,6 +1568,9 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
       }
     }
 
+    if (!r.m_uri.empty())
+      collectedUris.insert(r.m_uri);
+
     if (codecStr.empty())
       codecStr = CODEC::FOURCC_MP4A; // Fallback
 
@@ -1601,11 +1603,12 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
     }
   }
 
-  // Add subtitles renditions (do not take in account variants references)
+  // Add subtitles renditions
+  collectedUris.clear();
   for (const Rendition& r : pl.m_subtitleRenditions)
   {
     // There may be multiple renditions with the same uri but different GROUP-ID
-    if (r.m_isUriDuplicate)
+    if (collectedUris.contains(r.m_uri))
       continue;
 
     auto newAdpSet = CAdaptationSet::MakeUniquePtr(period.get());
@@ -1614,11 +1617,9 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
     if (!ParseRenditon(r, newAdpSet, newRepr))
       continue;
 
-    // Find the codec string from a variant that references it
-    const Variant* varFound = FindVariantBySubtitleGroupId(r.m_groupId, pl.m_variants);
     std::string codecStr;
-    if (varFound)
-      codecStr = GetSubtitleCodec(varFound->m_codecs);
+    if (r.m_variant)
+      codecStr = GetSubtitleCodec(r.m_variant->m_codecs);
     else
     {
       if (manifestCfg.hlsAddOrphansRenditions)
@@ -1634,6 +1635,9 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
       }
     }
 
+    if (!r.m_uri.empty())
+      collectedUris.insert(r.m_uri);
+
     if (codecStr.empty())
       codecStr = CODEC::FOURCC_WVTT; // WebVTT as default subtitle codec
 
@@ -1645,10 +1649,11 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
   }
 
   // Add variants
+  collectedUris.clear();
   for (const Variant& var : pl.m_variants)
   {
     // There may be multiple variants with the same uri but different AUDIO group
-    if (var.m_isUriDuplicate)
+    if (collectedUris.contains(var.m_uri))
       continue;
 
     if (var.m_bandwidth == 0)
@@ -1709,6 +1714,8 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
       if (!ParseRenditon(r, newAdpSet, newRepr))
         continue;
 
+      collectedUris.insert(var.m_uri);
+
       newAdpSet->AddCodecs(codecAudio);
       newRepr->AddCodecs(codecAudio);
       newRepr->SetBandwidth(var.m_bandwidth);
@@ -1781,6 +1788,8 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
       repr->max_buffer_duration_ = m_settings.m_bufferMaxDuration;
       repr->SetScaling();
 
+      collectedUris.insert(var.m_uri);
+
       std::string uri = var.m_uri;
       if (URL::IsUrlRelative(uri))
         uri = URL::Join(base_url_, uri);

src/parser/HLSTree.h

@@ -83,10 +83,13 @@ class ATTR_DLL_LOCAL CHLSTree : public AdaptiveTree
   };
 
   // \brief Usually refer to an EXT-X-MEDIA tag
+  struct Variant;
+
   struct Rendition
   {
     std::string m_type;
     std::string m_groupId;
+    const Variant* m_variant{nullptr}; // The first variant found related to GROUP-ID, if any
     std::string m_language;
     std::string m_name;
     bool m_isDefault{false};
@@ -95,7 +98,6 @@ class ATTR_DLL_LOCAL CHLSTree : public AdaptiveTree
     std::string m_characteristics;
     std::string m_uri;
     int m_features{REND_FEATURE_NONE};
-    bool m_isUriDuplicate{false}; // Another rendition have same uri
   };
 
   // \brief Usually refer to an EXT-X-STREAM-INF tag
@@ -109,7 +111,6 @@ class ATTR_DLL_LOCAL CHLSTree : public AdaptiveTree
     std::string m_groupIdSubtitles;
     std::string m_videoRange;
     std::string m_uri;
-    bool m_isUriDuplicate{false}; // Another variant have same uri
   };
 
   struct MultivariantPlaylist

src/test/TestHLSTree.cpp

@@ -50,7 +50,7 @@ class HLSTreeTest : public ::testing::Test
   {
     testHelper::testFile = filePath;
 
-    CSrvBroker::GetInstance()->InitStage1({});
+    CSrvBroker::GetInstance()->InitStage1(m_kodiProps);
 
     // Download the manifest
     UTILS::CURL::HTTPResponse resp;
@@ -93,8 +93,17 @@ class HLSTreeTest : public ::testing::Test
     return tree->PrepareRepresentation(per, adp, rep);
   }
 
+  // To set custom Kodi properties, must be called before OpenTestFile method
+  void SetKodiProps(bool hlsAddOrphansRenditions)
+  {
+    const std::string hlsAddOrphansRenditionsStr = hlsAddOrphansRenditions ? "true" : "false";
+    m_kodiProps.emplace("inputstream.adaptive.manifest_config",
+                        "{\"hls_add_orphans_renditions\":" + hlsAddOrphansRenditionsStr + "}");
+  }
+
   adaptive::CHLSTree* tree;
   CHOOSER::IRepresentationChooser* m_reprChooser{nullptr};
+  std::map<std::string, std::string> m_kodiProps;
 };
 
 
@@ -446,3 +455,66 @@ TEST_F(HLSTreeTest, FairPlaySkdUriProvidesClearKeyKid)
   EXPECT_EQ(drmInfos[0].defaultKid, "11111111123412341234000000000000");
   EXPECT_EQ(drmInfos[0].cryptoMode, CryptoMode::AES_CBC);
 }
+
+TEST_F(HLSTreeTest, ReferencedRenditionSharedUriWithOrphanGroup)
+{
+  // The orphaned group is listed first and shares its uri with the referenced one,
+  // the orphaned group should be skipped and the referenced one should be used.
+  SetKodiProps(false); // explicit hls_add_orphans_renditions to false, to ensure the test is not affected by the default value
+  OpenTestFileMaster("hls/1a1v_master_orphan_shared_uri.m3u8");
+
+  size_t audioAdpSets = 0;
+  for (const auto& adpSet : tree->m_periods[0]->GetAdaptationSets())
+  {
+    if (adpSet->GetStreamType() == PLAYLIST::StreamType::AUDIO)
+      audioAdpSets++;
+  }
+
+  EXPECT_EQ(audioAdpSets, 1);
+}
+
+TEST_F(HLSTreeTest, ReferencedRenditionOrphanGroup)
+{
+  // The orphaned group should be skipped and the referenced one should be used.
+  SetKodiProps(false); // explicit hls_add_orphans_renditions to false, to ensure the test is not affected by the default value
+  OpenTestFileMaster("hls/1a1v_master_orphan_audio_group.m3u8");
+
+  size_t audioAdpSets = 0;
+  PLAYLIST::CAdaptationSet* audioAdpSet{nullptr};
+
+  for (const auto& adpSet : tree->m_periods[0]->GetAdaptationSets())
+  {
+    if (adpSet->GetStreamType() == PLAYLIST::StreamType::AUDIO)
+    {
+      ++audioAdpSets;
+      audioAdpSet = adpSet.get();
+    }
+  }
+
+  EXPECT_EQ(audioAdpSets, 1);
+  if (audioAdpSet)
+  {
+    auto& repr = audioAdpSet->GetRepresentations()[0];
+    EXPECT_EQ(repr->GetSourceUrl(), "http://foo.bar/hls/stream_0/out_low.m3u8");
+  }
+}
+
+TEST_F(HLSTreeTest, ReferencedRenditionOrphanGroupIncluded)
+{
+  // The orphaned group is listed,
+  // the orphaned group should be included as set by hls_add_orphans_renditions.
+  SetKodiProps(true); // set hls_add_orphans_renditions to include orphaned renditions
+  OpenTestFileMaster("hls/1a1v_master_orphan_audio_group.m3u8");
+
+  size_t audioAdpSets = 0;
+
+  for (const auto& adpSet : tree->m_periods[0]->GetAdaptationSets())
+  {
+    if (adpSet->GetStreamType() == PLAYLIST::StreamType::AUDIO)
+    {
+      ++audioAdpSets;
+    }
+  }
+
+  EXPECT_EQ(audioAdpSets, 2);
+}

src/test/manifests/hls/1a1v_master_orphan_audio_group.m3u8

@@ -0,0 +1,6 @@
+#EXTM3U
+#EXT-X-VERSION:7
+#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="group_aud_orphan",NAME="audio_0",DEFAULT=YES,URI="stream_0/out_orphan.m3u8"
+#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="group_aud_low",NAME="audio_0",DEFAULT=YES,URI="stream_0/out_low.m3u8"
+#EXT-X-STREAM-INF:BANDWIDTH=1170400,RESOLUTION=1280x720,CODECS="avc1.64001f,mp4a.40.2",AUDIO="group_aud_low"
+stream_1/out.m3u8

src/test/manifests/hls/1a1v_master_orphan_shared_uri.m3u8

@@ -0,0 +1,6 @@
+#EXTM3U
+#EXT-X-VERSION:7
+#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="group_aud_orphan",NAME="audio_0",DEFAULT=YES,URI="stream_0/out.m3u8"
+#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="group_aud_low",NAME="audio_0",DEFAULT=YES,URI="stream_0/out.m3u8"
+#EXT-X-STREAM-INF:BANDWIDTH=1170400,RESOLUTION=1280x720,CODECS="avc1.64001f,mp4a.40.2",AUDIO="group_aud_low"
+stream_1/out.m3u8