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

[AdaptiveStream] Rework to improve HLS manifest updates #1459

Merged
merged 1 commit into from
Jan 28, 2024
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
35 changes: 21 additions & 14 deletions src/Session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -904,22 +904,29 @@ void CSession::UpdateStream(CStream& stream)

void CSession::PrepareStream(CStream* stream)
{
if (m_adaptiveTree->GetTreeType() != adaptive::TreeType::HLS)
return;

CRepresentation* repr = stream->m_adStream.getRepresentation();

switch (m_adaptiveTree->prepareRepresentation(stream->m_adStream.getPeriod(),
stream->m_adStream.getAdaptationSet(), repr))
if (stream->m_adStream.StreamChanged())
{
case PrepareRepStatus::FAILURE:
return;
case PrepareRepStatus::DRMCHANGED:
if (!InitializeDRM())
return;
[[fallthrough]];
case PrepareRepStatus::DRMUNCHANGED:
stream->m_isEncrypted = repr->m_psshSetPos != PSSHSET_POS_DEFAULT;
break;
default:
break;
// For HLS case when stream (representation) quality has been changed
// its not needed to update again the child manifest by calling ParseChildManifest
// because its already been done by AdaptiveStream thread worker or AdaptiveStream::ensureSegment
stream->m_isEncrypted = repr->GetPsshSetPos() != PSSHSET_POS_DEFAULT;
return;
}

bool isDrmChanged{false};
if (m_adaptiveTree->PrepareRepresentation(stream->m_adStream.getPeriod(),
stream->m_adStream.getAdaptationSet(), repr,
isDrmChanged))
{
if (isDrmChanged)
InitializeDRM();

stream->m_isEncrypted = repr->GetPsshSetPos() != PSSHSET_POS_DEFAULT;
}
}

Expand Down Expand Up @@ -1039,7 +1046,7 @@ bool CSession::GetNextSample(ISampleReader*& sampleReader)
waiting = stream.get();
break;
}
else if (!streamReader->EOS())
else if (streamReader->IsReady() && !streamReader->EOS())
{
if (AP4_SUCCEEDED(streamReader->Start(isStarted)))
{
Expand Down
80 changes: 55 additions & 25 deletions src/common/AdaptiveStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -720,15 +720,34 @@ bool AdaptiveStream::start_stream(uint64_t startPts)
current_rep_->get_segment(static_cast<size_t>(segmentId - current_rep_->GetStartNumber()));
}
}
else if (stream_changed_) // switching streams, align new stream segment no.
Copy link
Collaborator Author

@CastagnaIT CastagnaIT Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have lost most of the time due to this method AdaptiveStream::start_stream
because i was not aware that the AdaptiveStream::start_stream method is used to adjust/set the "current segment" and so all my changes that i was doing on ensure_segment always was failing.
After discovered this, i realized that was missing set "current segment" for a stream quality switching case

{
uint64_t segmentId = segment_buffers_[0]->segment_number;
if (segmentId >= current_rep_->GetStartNumber() + current_rep_->SegmentTimeline().GetSize())
{
segmentId = current_rep_->GetStartNumber() + current_rep_->SegmentTimeline().GetSize() - 1;
}
current_rep_->current_segment_ = current_rep_->get_segment(
static_cast<size_t>(segmentId - current_rep_->GetStartNumber()));
}
else
{
current_rep_->current_segment_ = nullptr; // start from beginning
}
}

stream_changed_ = false;

const CSegment* next_segment =
current_rep_->get_next_segment(current_rep_->current_segment_);

if (!next_segment)
{
//! @todo: THIS MUST BE CHANGED - !! BUG !!
//! this will broken/stop playback on live streams when adaptive stream change stream quality (so representation)
//! and there are no new segments because not immediately available (despite child manifest updated),
//! because we cant be ensure next segment without wait the appropriate timing...imo its not the right place here.
//! Can be reproduced with HLS live and by using "Stream selection" setting to "Test" by switching per 1 segment
Comment on lines +746 to +750
Copy link
Collaborator Author

@CastagnaIT CastagnaIT Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also found this problem (read todo) i tried force remove this block of code that stop playback, and a bit below avoid set currentPTSOffset_ when there is no next_segment, playback works but show weird pts on Kodi GUI the value of time becomes sometimes negative and it can lead to weirdness in the long run.
Need to be changed on other way but idk atm, can be done separately, maybe can you help here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the scenario makes sense why it fails. We could try a hacky workaround until something better comes along - assume when the next segment comes it will be the same size (usually is) and add that PTS to current segment's start PTS. Sound good or not?

Copy link
Collaborator Author

@CastagnaIT CastagnaIT Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i understood but as you said its hacky im not 100% sure that could be so good

i found this problem even when trying to fix bugs to make this live sample playable
https://livesim.dashif.org/livesim/periods_20/testpic_2s/Manifest.mpd

This live sample have periods but no new segments provided,
then its required the use of "InsertLiveSegment" in conjuction with the manifest update worker.
This sample somewhat works on kodi <=v20 because of old code hacks and periods seem "ignored",
on kodi 21 is ofc broken, from what i see seem there are multiple problems on CDashTree::RefreshLiveSegments that seem dont take in account of "inserted" segments on timeline, delete them all and so seem to loose the last inserted segment with the updated pts (because you have to take in account that the downloaded updated manifest dont provide new segments), this will make problems with AdaptiveStream::ensureSegment that its not able to find next segment, AdaptiveStream::start_stream have problem when switch period, and also with CSession::OnSegmentChangedRead that its not able to insert new segments anymore
(these problems may be also similar with #1461)

absolute_position_ = ~0;
state_ = STOPPED;
return true;
Expand Down Expand Up @@ -772,11 +791,11 @@ bool AdaptiveStream::start_stream(uint64_t startPts)
return false;
}

currentPTSOffset_ = (next_segment->startPTS_ * current_rep_->timescale_ext_) /
current_rep_->timescale_int_;
currentPTSOffset_ =
(next_segment->startPTS_ * current_rep_->timescale_ext_) / current_rep_->timescale_int_;
absolutePTSOffset_ =
(current_rep_->SegmentTimeline().Get(0)->startPTS_ * current_rep_->timescale_ext_) /
current_rep_->timescale_int_;
current_rep_->timescale_int_;

if (state_ == RUNNING)
{
Expand Down Expand Up @@ -838,7 +857,6 @@ bool AdaptiveStream::ensureSegment()
if (m_fixateInitialization)
return false;

stream_changed_ = false;
CSegment* nextSegment{nullptr};
last_rep_ = current_rep_;

Expand All @@ -849,16 +867,18 @@ bool AdaptiveStream::ensureSegment()
segment_buffers_.begin() + available_segment_buffers_);
--valid_segment_buffers_;
--available_segment_buffers_;
// Check if next segment use same representation of previous one
// if not, update the current representation
// Adaptive stream has changed quality (and so changed representation)
if (segment_buffers_[0]->rep != current_rep_)
{
current_rep_->SetIsEnabled(false);
current_rep_ = segment_buffers_[0]->rep;
current_rep_->SetIsEnabled(true);
// When stream changed flag is signalled, kodi reopen the stream and AdaptiveStream::start_stream method
// will be called also to align the "current segment" for the current representation
stream_changed_ = true;
}
}

if (valid_segment_buffers_ > 0)
{
if (!segment_buffers_[0]->segment.IsInitialization())
Expand Down Expand Up @@ -910,20 +930,25 @@ bool AdaptiveStream::ensureSegment()
newRep = prevRep;
else
newRep = m_tree->GetRepChooser()->GetNextRepresentation(current_adp_, prevRep);
}

// If the representation has been changed, segments may have to be generated (DASH)
if (newRep->SegmentTimeline().IsEmpty())
GenerateSidxSegments(newRep);

if (!newRep->IsPrepared() && m_tree->SecondsSinceRepUpdate(newRep) > 1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On representation class IsPrepared and m_isDownloaded plus the SecondsSinceRepUpdate imo was really a mess of behaviours, i have removed them all, and add a new IsNeedsUpdates, used only internally in the HLSParser to distinguish whether a representation needs an update of some thing (in this case for live updates)

{
m_tree->prepareRepresentation(current_period_, current_adp_, newRep, m_tree->IsLive());
if (newRep != prevRep) // Stream quality changed
{
// On manifests type like HLS we need also to get update segments
// because need to be downloaded/parsed from different child manifest files
bool isDrmChanged;
m_tree->PrepareRepresentation(current_period_, current_adp_, newRep, isDrmChanged);

// If the representation has been changed, segments may have to be generated (DASH)
if (newRep->SegmentTimeline().IsEmpty())
GenerateSidxSegments(newRep);
}
}

// Add to the buffer next future segment
// Add to the buffer next segment (and the next following segments, if available)

// Align to new segment numeration (that could be changed if manifest update occurred with quality switching)
size_t nextsegmentPos = static_cast<size_t>(nextsegno - newRep->GetStartNumber());

if (nextsegmentPos + available_segment_buffers_ >= newRep->SegmentTimeline().GetSize())
{
nextsegmentPos = newRep->SegmentTimeline().GetSize() - available_segment_buffers_;
Expand Down Expand Up @@ -963,7 +988,8 @@ bool AdaptiveStream::ensureSegment()
if (!current_rep_->IsWaitForSegment())
{
current_rep_->SetIsWaitForSegment(true);
LOG::LogF(LOGDEBUG, "[AS-%u] Begin WaitForSegment stream %s", clsId, current_rep_->GetId().data());
LOG::LogF(LOGDEBUG, "[AS-%u] Begin WaitForSegment stream id \"%s\"", clsId,
current_rep_->GetId().data());
}
std::this_thread::sleep_for(std::chrono::milliseconds(100));
return false;
Expand Down Expand Up @@ -1212,17 +1238,22 @@ size_t adaptive::AdaptiveStream::getSegmentPos()

bool AdaptiveStream::waitingForSegment(bool checkTime) const
{
if (current_adp_->GetStreamType() == StreamType::SUBTITLE)
return false;

if ((m_tree->HasManifestUpdates() || m_tree->HasManifestUpdatesSegs()) && state_ == RUNNING)
{
std::lock_guard<adaptive::AdaptiveTree::TreeUpdateThread> lckUpdTree(m_tree->GetTreeUpdMutex());

if (current_rep_ && current_rep_->IsWaitForSegment())
{
return !checkTime ||
(current_adp_->GetStreamType() != StreamType::VIDEO &&
current_adp_->GetStreamType() != StreamType::AUDIO) ||
SecondsSinceUpdate() < 1;
Copy link
Collaborator Author

@CastagnaIT CastagnaIT Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SecondsSinceUpdate uses (now removed) above and here should be the cause of the RAI buzz/frezze sound, by debugging this waitingForSegment method, was producing messed up return values

return !checkTime || (current_adp_->GetStreamType() != StreamType::VIDEO &&
current_adp_->GetStreamType() != StreamType::AUDIO);
}

// We can fall here when we are waiting for new segments, but the download it not started yet
// so return true only when checkTime is false, to avoid trigger EOS on sample reader and stop playback
return !checkTime;
Copy link
Collaborator Author

@CastagnaIT CastagnaIT Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main cause because the Pluto TV stream was stopping playback (before was returning "false")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

}
return false;
}
Expand Down Expand Up @@ -1289,16 +1320,15 @@ bool AdaptiveStream::GenerateSidxSegments(PLAYLIST::CRepresentation* rep)

void AdaptiveStream::Stop()
{
if (current_rep_)
{
current_rep_->SetIsEnabled(false);
}

if (thread_data_)
{
thread_data_->Stop();
StopWorker(STOPPED);
}
// Disable representation only after stopped the worker
// otherwise if read some segments may invalidate this change
if (current_rep_)
current_rep_->SetIsEnabled(false);
Comment on lines +1328 to +1331
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the problem should be due to AdaptiveStream::ensureSegment
since force change SetIsEnabled on the representation and race condition may happens

}

void AdaptiveStream::clear()
Expand Down
1 change: 1 addition & 0 deletions src/common/AdaptiveStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ class AdaptiveTree;
PLAYLIST::CPeriod* current_period_;
PLAYLIST::CAdaptationSet* current_adp_;
PLAYLIST::CRepresentation* current_rep_;
PLAYLIST::CRepresentation* m_switchRep{nullptr};

// Decrypter IV used to decrypt HLS segment
// We need to store here because linked to representation
Expand Down
14 changes: 9 additions & 5 deletions src/common/AdaptiveTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,16 @@ class ATTR_DLL_LOCAL AdaptiveTree
*/
virtual void PostOpen();

virtual PLAYLIST::PrepareRepStatus prepareRepresentation(PLAYLIST::CPeriod* period,
PLAYLIST::CAdaptationSet* adp,
PLAYLIST::CRepresentation* rep,
bool update = false)
/*!
* \brief Prepare the representation data by downloading and parsing manifest
* \return True if has success, otherwise false
*/
virtual bool PrepareRepresentation(PLAYLIST::CPeriod* period,
PLAYLIST::CAdaptationSet* adp,
PLAYLIST::CRepresentation* rep,
bool& isDrmChanged)
{
return PLAYLIST::PrepareRepStatus::OK;
return false;
}

virtual std::chrono::time_point<std::chrono::system_clock> GetRepLastUpdated(
Expand Down
8 changes: 0 additions & 8 deletions src/common/AdaptiveUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ enum class EncryptionType
UNKNOWN,
};

enum class PrepareRepStatus
{
FAILURE,
OK,
DRMCHANGED,
DRMUNCHANGED,
};

enum class ContainerType
{
NOTYPE,
Expand Down
1 change: 0 additions & 1 deletion src/common/Representation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,5 @@ void PLAYLIST::CRepresentation::CopyHLSData(const CRepresentation* other)
m_isIncludedStream = other->m_isIncludedStream;
m_isEnabled = other->m_isEnabled;
m_isWaitForSegment = other->m_isWaitForSegment;
m_isDownloaded = other->m_isDownloaded;
m_initSegment = other->m_initSegment;
}
13 changes: 4 additions & 9 deletions src/common/Representation.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,6 @@ class ATTR_DLL_LOCAL CRepresentation : public CCommonSegAttribs, public CCommonA
m_isSubtitleFileStream = isSubtitleFileStream;
}

// Currently used for HLS only
bool IsPrepared() const { return m_isPrepared; }
// Currently used for HLS only
void SetIsPrepared(bool isPrepared) { m_isPrepared = isPrepared; }

bool IsEnabled() const { return m_isEnabled; }
void SetIsEnabled(bool isEnabled) { m_isEnabled = isEnabled; }

Expand All @@ -151,6 +146,9 @@ class ATTR_DLL_LOCAL CRepresentation : public CCommonSegAttribs, public CCommonA
bool IsIncludedStream() const { return m_isIncludedStream; }
void SetIsIncludedStream(bool isIncludedStream) { m_isIncludedStream = isIncludedStream; }

bool IsNeedsUpdates() const { return m_isNeedsUpdates; }
void SetIsNeedsUpdates(bool isNeedsUpdates) { m_isNeedsUpdates = isNeedsUpdates; }

void CopyHLSData(const CRepresentation* other);

static bool CompareBandwidth(std::unique_ptr<CRepresentation>& left,
Expand Down Expand Up @@ -259,9 +257,6 @@ class ATTR_DLL_LOCAL CRepresentation : public CCommonSegAttribs, public CCommonA
uint32_t assured_buffer_duration_{0};
uint32_t max_buffer_duration_{0};

//! @todo: used for HLS only, not reflect the right meaning
bool m_isDownloaded{false};

protected:
std::string m_id;
std::string m_sourceUrl;
Expand All @@ -287,11 +282,11 @@ class ATTR_DLL_LOCAL CRepresentation : public CCommonSegAttribs, public CCommonA

bool m_isSubtitleFileStream{false};

bool m_isPrepared{false};
bool m_isEnabled{false};
bool m_isWaitForSegment{false};

bool m_isIncludedStream{false};
bool m_isNeedsUpdates{true};
};

} // namespace PLAYLIST
20 changes: 18 additions & 2 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ void CInputStreamAdaptive::EnableStream(int streamid, bool enable)
bool CInputStreamAdaptive::OpenStream(int streamid)
{
LOG::Log(LOGDEBUG, "OpenStream(%d)", streamid);
// This method can be called when:
// - Stream first start
// - Chapter/period change
// - Automatic stream (representation) quality change (such as adaptive)
// - Manual stream (representation) quality change (from OSD)

if (!m_session)
return false;
Expand All @@ -226,6 +231,12 @@ bool CInputStreamAdaptive::OpenStream(int streamid)

stream->m_isEnabled = true;

//! @todo: for live with multiple periods (like HLS DISCONTINUITIES) for subtitle case
//! when the subtitle has been disabled from video start, and happens a period change,
//! kodi call OpenStream to the subtitle stream as if it were enabled, and disable it with EnableStream
//! just after it, this lead to log error "GenerateSidxSegments: [AS-x] Cannot generate segments from SIDX on repr..."
//! need to find a solution to avoid open the stream when previously disabled from previous period

CRepresentation* rep = stream->m_adStream.getRepresentation();

// If we select a dummy (=inside video) stream, open the video part
Expand Down Expand Up @@ -342,10 +353,11 @@ DEMUX_PACKET* CInputStreamAdaptive::DemuxRead(void)

if (m_session->CheckChange())
{
// Adaptive stream has switched stream (representation) quality
m_lastPts = PLAYLIST::NO_PTS_VALUE;
p = AllocateDemuxPacket(0);
p->iStreamId = DEMUX_SPECIALID_STREAMCHANGE;
LOG::Log(LOGDEBUG, "DEMUX_SPECIALID_STREAMCHANGE");
LOG::Log(LOGDEBUG, "DEMUX_SPECIALID_STREAMCHANGE (stream quality changed)");
return p;
}

Expand Down Expand Up @@ -404,8 +416,12 @@ DEMUX_PACKET* CInputStreamAdaptive::DemuxRead(void)
return p;
}

// Ends here when GetNextSample fails due to sample reader in EOS state or stream disabled
// that could means, in case of multi-periods streams, that segments are ended
// in the current period and its needed to switch to the next period
if (m_session->SeekChapter(m_session->GetChapter() + 1))
{
// Switched to new period / chapter
m_checkChapterSeek = true;
m_lastPts = PLAYLIST::NO_PTS_VALUE;
for (unsigned int i(1);
Expand All @@ -414,7 +430,7 @@ DEMUX_PACKET* CInputStreamAdaptive::DemuxRead(void)
m_session->InitializePeriod();
DEMUX_PACKET* p = AllocateDemuxPacket(0);
p->iStreamId = DEMUX_SPECIALID_STREAMCHANGE;
LOG::Log(LOGDEBUG, "DEMUX_SPECIALID_STREAMCHANGE");
LOG::Log(LOGDEBUG, "DEMUX_SPECIALID_STREAMCHANGE (chapter changed)");
return p;
}
return NULL;
Expand Down
1 change: 0 additions & 1 deletion src/parser/DASHTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,6 @@ void adaptive::CDashTree::RefreshSegments(PLAYLIST::CPeriod* period,
{
if (adp->GetStreamType() == StreamType::VIDEO || adp->GetStreamType() == StreamType::AUDIO)
{
m_updThread.ResetStartTime();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be a remnant of the old manifest update code, has no sense reset the worker thread here, isnt used when we update manifest at each new segment request "manually"

RefreshLiveSegments();
}
}
Expand Down
Loading
Loading