Skip to content

Commit

Permalink
Make TimeSeek multithread safe / Fix segfault when AP4_Sample's seek …
Browse files Browse the repository at this point in the history
…fails (double free)
  • Loading branch information
peak3d committed Aug 12, 2017
1 parent a98e877 commit 0e6bb85
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 11 deletions.
1 change: 1 addition & 0 deletions lib/libbento4/Core/Ap4LinearReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ AP4_LinearReader::Advance(bool read_data)
result = buffer->m_Sample->ReadData(buffer->m_Data);
}
if (AP4_FAILED(result)) {
buffer->m_Sample = nullptr;
delete buffer;
return result;
}
Expand Down
32 changes: 26 additions & 6 deletions src/common/AdaptiveStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ bool AdaptiveStream::ensureSegment()

if (!loading_seg_ && segment_read_pos_ >= segment_buffer_.size())
{
//wait until worker is reeady for new segment
std::lock_guard<std::mutex> lck(thread_data_->mutex_dl_);
current_seg_ = current_rep_->get_next_segment(current_seg_);
if (current_seg_)
{
Expand All @@ -285,6 +287,7 @@ uint32_t AdaptiveStream::read(void* buffer, uint32_t bytesToRead)
{
std::unique_lock<std::mutex> lckrw(thread_data_->mutex_rw_);

NEXTSEGMENT:
if (ensureSegment() && bytesToRead)
{
while (true)
Expand All @@ -307,6 +310,9 @@ uint32_t AdaptiveStream::read(void* buffer, uint32_t bytesToRead)
memcpy(buffer, segment_buffer_.data() + (segment_read_pos_ - avail), avail);
return avail;
}
// If we call read after the last chunk was read but before worker finishes download, we end up here.
if (!avail)
goto NEXTSEGMENT;
return 0;
}
}
Expand All @@ -315,11 +321,15 @@ uint32_t AdaptiveStream::read(void* buffer, uint32_t bytesToRead)

bool AdaptiveStream::seek(uint64_t const pos)
{
std::lock_guard<std::mutex> lckrw(thread_data_->mutex_rw_);
std::unique_lock<std::mutex> lckrw(thread_data_->mutex_rw_);
// we seek only in the current segment
if (pos >= absolute_position_ - segment_read_pos_)
{
segment_read_pos_ = static_cast<uint32_t>(pos - (absolute_position_ - segment_read_pos_));

while (segment_read_pos_ > segment_buffer_.size() && loading_seg_)
thread_data_->signal_rw_.wait(lckrw);

if (segment_read_pos_ > segment_buffer_.size())
{
segment_read_pos_ = static_cast<uint32_t>(segment_buffer_.size());
Expand All @@ -333,7 +343,7 @@ bool AdaptiveStream::seek(uint64_t const pos)

bool AdaptiveStream::seek_time(double seek_seconds, double current_seconds, bool &needReset)
{
if (!current_rep_)
if (!current_rep_ || stopped_)
return false;

if (current_rep_->flags_ & AdaptiveTree::Representation::SUBTITLESTREAM)
Expand All @@ -352,12 +362,22 @@ bool AdaptiveStream::seek_time(double seek_seconds, double current_seconds, bool
if (choosen_seg && current_rep_->get_segment(choosen_seg)->startPTS_ > sec_in_ts)
--choosen_seg;

const AdaptiveTree::Segment* old_seg(current_seg_);
if ((current_seg_ = current_rep_->get_segment(choosen_seg)))
const AdaptiveTree::Segment *old_seg(current_seg_), *newSeg(current_rep_->get_segment(choosen_seg));
if (newSeg)
{
needReset = true;
if (current_seg_ != old_seg)
download_segment();
if (newSeg != old_seg)
{
//stop downloading chunks
stopped_ = true;
//wait until last reading operation stopped
std::lock_guard<std::mutex> lck(thread_data_->mutex_dl_);
stopped_ = false;
current_seg_ = loading_seg_ = newSeg;
absolute_position_ = 0;
ResetSegment();
thread_data_->signal_dl_.notify_one();
}
else if (seek_seconds < current_seconds)
{
absolute_position_ -= segment_read_pos_;
Expand Down
5 changes: 4 additions & 1 deletion src/common/AdaptiveTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,14 @@ namespace adaptive
return 0;
}

void AdaptiveTree::SortRepresentations()
void AdaptiveTree::SortTree()
{
for (std::vector<Period*>::const_iterator bp(periods_.begin()), ep(periods_.end()); bp != ep; ++bp)
{
std::sort((*bp)->adaptationSets_.begin(), (*bp)->adaptationSets_.end(), AdaptationSet::compare);
for (std::vector<AdaptationSet*>::const_iterator ba((*bp)->adaptationSets_.begin()), ea((*bp)->adaptationSets_.end()); ba != ea; ++ba)
std::sort((*ba)->repesentations_.begin(), (*ba)->repesentations_.end(), Representation::compare);
}
}

} // namespace
4 changes: 3 additions & 1 deletion src/common/AdaptiveTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ namespace adaptive
return *segment_durations_[pos];
};

static bool compare(const AdaptationSet* a, const AdaptationSet *b) { return a->type_ < b->type_; };

SegmentTemplate segtpl_;
}*current_adaptationset_;

Expand Down Expand Up @@ -281,7 +283,7 @@ namespace adaptive
protected:
virtual bool download(const char* url, const std::map<std::string, std::string> &manifestHeaders);
virtual bool write_data(void *buffer, size_t buffer_size) = 0;
void SortRepresentations();
void SortTree();
private:
std::mutex m_mutex;
};
Expand Down
2 changes: 2 additions & 0 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,8 @@ bool Session::SeekTime(double seekTime, unsigned int streamId, bool preceeding)
else
{
kodi::Log(ADDON_LOG_INFO, "seekTime(%0.4f) for Stream:%d continues at %llu", seekTime, (*b)->info_.m_pID, (*b)->reader_->PTS());
if ((*b)->info_.m_streamType == INPUTSTREAM_INFO::TYPE_VIDEO)
seekTime = static_cast<double>((*b)->reader_->PTS()) / DVD_TIME_BASE, preceeding = false;
ret = true;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/parser/DASHTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ bool DASHTree::open(const char *url)
XML_ParserFree(parser_);
parser_ = 0;

SortRepresentations();
SortTree();

return ret;
}
Expand Down
2 changes: 1 addition & 1 deletion src/parser/HLSTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ bool HLSTree::open(const char *url)
current_period_->adaptationSets_.push_back(adp);
m_extGroups.clear();

SortRepresentations();
SortTree();
}
// Set Live as default
has_timeshift_buffer_ = true;
Expand Down
2 changes: 1 addition & 1 deletion src/parser/SmoothTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ bool SmoothTree::open(const char *url)
}
}

SortRepresentations();
SortTree();

return true;
}
Expand Down

0 comments on commit 0e6bb85

Please sign in to comment.