Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HLSTree] Fixes for parser regressions #1364

Merged
merged 3 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/parser/DASHTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ bool adaptive::CDashTree::Open(std::string_view url,

m_manifestRespHeaders = headers;
manifest_url_ = url;
base_url_ = URL::RemoveParameters(url.data());
base_url_ = URL::GetUrlPath(url.data());

if (!ParseManifest(data))
return false;
Expand Down Expand Up @@ -1514,7 +1514,7 @@ void adaptive::CDashTree::RefreshLiveSegments()
{
manifestUrl = manifest_url_;
if (!manifestParams.empty())
manifestUrl = URL::RemoveParameters(manifestUrl, false);
manifestUrl = URL::RemoveParameters(manifestUrl);
}
else
manifestUrl = location_;
Expand Down
47 changes: 43 additions & 4 deletions src/parser/HLSTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "../utils/log.h"
#include "kodi/tools/StringUtils.h"

#include <algorithm>
#include <optional>
#include <sstream>

Expand Down Expand Up @@ -151,7 +152,7 @@ bool adaptive::CHLSTree::Open(std::string_view url,
SaveManifest(nullptr, data, url);

manifest_url_ = url;
base_url_ = URL::RemoveParameters(url.data());
base_url_ = URL::GetUrlPath(url.data());

if (!ParseManifest(data))
{
Expand Down Expand Up @@ -205,7 +206,7 @@ PLAYLIST::PrepareRepStatus adaptive::CHLSTree::prepareRepresentation(PLAYLIST::C

SaveManifest(adp, resp.data, manifestUrl);

rep->SetBaseUrl(URL::RemoveParameters(resp.effectiveUrl));
rep->SetBaseUrl(URL::GetUrlPath(resp.effectiveUrl));

EncryptionType currentEncryptionType = EncryptionType::CLEAR;

Expand Down Expand Up @@ -354,7 +355,7 @@ PLAYLIST::PrepareRepStatus adaptive::CHLSTree::prepareRepresentation(PLAYLIST::C
if (rep->GetContainerType() == ContainerType::NOTYPE)
{
// Try find the container type on the representation according to the file extension
std::string url = URL::RemoveParameters(line, false);
std::string url = URL::RemoveParameters(line);
// Remove domain on absolute url, to not confuse top-level domain as extension
url = url.substr(URL::GetDomainUrl(url).size());

Expand Down Expand Up @@ -488,11 +489,17 @@ PLAYLIST::PrepareRepStatus adaptive::CHLSTree::prepareRepresentation(PLAYLIST::C
m_periods.push_back(std::move(newPeriod));
}
else
{
// Period(s) already created by a previously downloaded manifest child
period = m_periods[discontCount].get();
}

newStartNumber += rep->SegmentTimeline().GetSize();
adp = period->GetAdaptationSets()[adpSetPos].get();
rep = adp->GetRepresentations()[reprPos].get();
// When we switch to a repr of another period we need to set current base url
CRepresentation* switchRep = adp->GetRepresentations()[reprPos].get();
switchRep->SetBaseUrl(rep->GetBaseUrl());
rep = switchRep;

currentSegStartPts = 0;

Expand Down Expand Up @@ -974,6 +981,19 @@ 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);
Expand Down Expand Up @@ -1017,6 +1037,13 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
var.m_groupIdSubtitles = attribs["SUBTITLES"];
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);
}
else if (tagName == "#EXT-X-SESSION-KEY")
Expand Down Expand Up @@ -1050,6 +1077,10 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
// Add audio renditions (do not take in account variants references)
for (const Rendition& r : pl.m_audioRenditions)
{
// There may be multiple renditions with the same uri but different GROUP-ID
if (r.m_isUriDuplicate)
continue;

auto newAdpSet = CAdaptationSet::MakeUniquePtr(period.get());
auto newRepr = CRepresentation::MakeUniquePtr(newAdpSet.get());

Expand Down Expand Up @@ -1096,6 +1127,10 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
// Add subtitles renditions (do not take in account variants references)
for (const Rendition& r : pl.m_subtitleRenditions)
{
// There may be multiple renditions with the same uri but different GROUP-ID
if (r.m_isUriDuplicate)
continue;

auto newAdpSet = CAdaptationSet::MakeUniquePtr(period.get());
auto newRepr = CRepresentation::MakeUniquePtr(newAdpSet.get());

Expand All @@ -1112,6 +1147,10 @@ bool adaptive::CHLSTree::ParseMultivariantPlaylist(const std::string& data)
// Add variants
for (const Variant& var : pl.m_variants)
{
// There may be multiple variants with the same uri but different AUDIO group
if (var.m_isUriDuplicate)
continue;

if (var.m_bandwidth == 0)
LOG::LogF(LOGWARNING, "Variant with malformed bandwidth attribute");

Expand Down
2 changes: 2 additions & 0 deletions src/parser/HLSTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ 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
Expand All @@ -91,6 +92,7 @@ class ATTR_DLL_LOCAL CHLSTree : public AdaptiveTree
std::string m_groupIdAudio;
std::string m_groupIdSubtitles;
std::string m_uri;
bool m_isUriDuplicate{false}; // Another variant have same uri
};

struct MultivariantPlaylist
Expand Down
2 changes: 1 addition & 1 deletion src/parser/SmoothTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ bool adaptive::CSmoothTree::Open(std::string_view url,
SaveManifest("", data, "");

manifest_url_ = url;
base_url_ = URL::RemoveParameters(url.data());
base_url_ = URL::GetUrlPath(url.data());

if (!ParseManifest(data))
return false;
Expand Down
24 changes: 19 additions & 5 deletions src/utils/UrlUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,18 +164,32 @@ std::string UTILS::URL::GetParameters(std::string& url) {
return "";
}

std::string UTILS::URL::RemoveParameters(std::string url, bool removeFilenameParam /* = true */)
std::string UTILS::URL::RemoveParameters(std::string url)
{
size_t paramsPos = url.find('?');
if (paramsPos != std::string::npos)
url.resize(paramsPos);

if (removeFilenameParam)
return url;
}

std::string UTILS::URL::GetUrlPath(std::string url)
{
if (url.empty())
return url;

size_t paramsPos = url.find('?');
if (paramsPos != std::string::npos)
url.resize(paramsPos);

// The part of the base url after last / is not a directory so will not be taken into account
if (url.back() != '/')
{
size_t slashPos = url.find_last_of('/');
if (slashPos != std::string::npos && slashPos != (url.find("://") + 2))
url.resize(slashPos + 1);
size_t slashPos = url.rfind("/");
if (slashPos > url.find("://") + 3)
url.erase(slashPos + 1);
}

return url;
}

Expand Down
19 changes: 13 additions & 6 deletions src/utils/UrlUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,20 @@ std::string GetParametersFromPlaceholder(std::string& url, std::string_view plac
*/
std::string GetParameters(std::string& url);

/*! \brief Remove URL parameters e.g. "?q=something"
* \param url An URL
* \param removeFilenameParam If true remove the last URL level if it
* contains a filename with extension
* \return The URL without parameters
/*!
* \brief Remove URL parameters e.g. "?q=something"
* \param url An URL
* \return The URL without parameters
*/
std::string RemoveParameters(std::string url);

/*!
* \brief Get the url path, by removing file part and parameters
* e.g. https://sample.com/part1/part2?test become https://sample.com/part1/
* \param url An URL
* \return The URL path
*/
std::string RemoveParameters(std::string url, bool removeFilenameParam = true);
std::string GetUrlPath(std::string url);

/*! \brief Append a string of parameters to an URL with/without pre-existents params
* \param url URL where append the parameters
Expand Down
Loading