wip

Author: CastagnaIT

src/Session.cpp

@@ -717,6 +717,14 @@ bool SESSION::CSession::PrepareStream(CStream& stream, uint64_t startPts)
     }
   }
 
+  //! @todo: when the stream will be opened (OpenStream) while you change chapter/period during video seek,
+  //!  triggered by DEMUX_SPECIALID_STREAMCHANGE (chapter changed)
+  //!  the adaptive stream start to download segments from the start of period, for nothing,
+  //!  because when CInputStreamAdaptive::DemuxRead call CSession::OnDemuxRead
+  //!  will call Session::SeekTime method that cause to delete and cancel current downloads/buffer,
+  //!  and re-start downloads from the appropriate PTS,
+  //!  this wastes time for the video seek and resources due to unnecessary downloads
+  //!  more likely its more easy to see also cancelled downloads on log with TS streams
   stream.m_adStream.start_stream(startPts);
   stream.SetAdByteStream(std::make_unique<CAdaptiveByteStream>(&stream.m_adStream));
 
@@ -839,35 +847,6 @@ uint64_t SESSION::CSession::GetTimeshiftBufferStart()
     return 0ULL;
 }
 
-// TODO: clean this up along with seektime
-void SESSION::CSession::StartReader(
-    CStream* stream, uint64_t seekTime, int64_t ptsDiff, bool preceeding, bool timing)
-{
-  ISampleReader* streamReader = stream->GetReader();
-  if (!streamReader)
-  {
-    LOG::LogF(LOGERROR, "Cannot get the stream reader");
-    return;
-  }
-
-  bool bReset = true;
-  if (timing)
-    seekTime += stream->m_adStream.GetAbsolutePTSOffset();
-  else
-    seekTime -= ptsDiff;
-
-  stream->m_adStream.seek_time(static_cast<double>(seekTime / STREAM_TIME_BASE), preceeding,
-                               bReset);
-
-  if (bReset)
-    streamReader->Reset(false);
-
-  bool bStarted = false;
-  streamReader->Start(bStarted);
-  if (bStarted && (streamReader->GetInformation(stream->m_info)))
-    m_changed = true;
-}
-
 void SESSION::CSession::OnScreenResChange()
 {
   m_reprChooser->OnUpdateScreenRes();
@@ -992,23 +971,64 @@ bool SESSION::CSession::SeekTime(double seekTime, bool preceeding)
       seekTime = maxSeek;
   }
 
+  // Helper lambda to check if reader is started,
+  // since after seeking across chapters/periods the reader is not started yet
+  auto CheckReaderRunning = [this](CStream& stream) -> bool
+  {
+    auto reader = stream.GetReader();
+    if (!reader->IsStarted())
+    {
+      bool isStarted = false;
+      if (AP4_FAILED(reader->Start(isStarted)))
+        return false;
+
+      if (isStarted && reader->GetInformation(stream.m_info))
+        m_changed = true;
+    }
+    return true;
+  };
+
+  // Helper lambda to perform seek on adaptive stream segment buffer
+  auto SeekAdStream = [&preceeding](CStream& stream, double seekSecs) -> bool
+  {
+    bool reset = false;
+    if (!stream.m_adStream.seek_time(seekSecs, preceeding, reset))
+    {
+      stream.GetReader()->Reset(true);
+      return false;
+    }
+    if (reset)
+      stream.GetReader()->Reset(false);
+
+    return true;
+  };
+
   // correct for starting segment pts value of chapter and chapter offset within program
   uint64_t seekTimeCorrected{static_cast<uint64_t>(seekTime * STREAM_TIME_BASE)};
   int64_t ptsDiff{0};
+
+  // If we have a timing stream, ensure it is started first and compute ptsDiff /
+  // corrected seek base that will be used by all other streams.
   if (m_timingStream)
   {
-    // after seeking across chapters with fmp4 streams the reader will not have started
-    // so we start here to ensure that we have the required information to correctly
-    // seek with proper stream alignment
     ISampleReader* timingReader{m_timingStream->GetReader()};
     if (!timingReader)
     {
-      LOG::LogF(LOGERROR, "Cannot get the stream sample reader");
+      LOG::LogF(LOGERROR, "Cannot get the stream sample reader of timing stream");
       return false;
     }
+
     timingReader->WaitReadSampleAsyncComplete();
-    if (!timingReader->IsStarted())
-      StartReader(m_timingStream.get(), seekTimeCorrected, ptsDiff, preceeding, true);
+
+    double seekSecs =
+        static_cast<double>(seekTimeCorrected + m_timingStream->m_adStream.GetAbsolutePTSOffset()) /
+        STREAM_TIME_BASE;
+
+    if (!SeekAdStream(*m_timingStream, seekSecs))
+      return false;
+
+    if (!CheckReaderRunning(*m_timingStream))
+      return false;
 
     seekTimeCorrected += m_timingStream->m_adStream.GetAbsolutePTSOffset();
     ptsDiff = timingReader->GetPTSDiff();
@@ -1025,54 +1045,50 @@ bool SESSION::CSession::SeekTime(double seekTime, bool preceeding)
       continue;
 
     streamReader->WaitReadSampleAsyncComplete();
-    if (stream->IsEnabled())
+    if (!stream->IsEnabled())
+      continue;
+
+    double seekSecs{static_cast<double>(seekTimeCorrected - ptsDiff) / STREAM_TIME_BASE};
+    bool reset{false};
+
+    // With FMP4 audio stream included to video stream you must not seek with AdaptiveStream
+    // segments are managed by AdaptiveStream of the video stream
+    // so CFragmentedSampleReader read data from the reader of the video stream
+    const bool hasAdStream = !(streamReader->GetType() == ISampleReader::Type::FMP4 &&
+                               stream->m_adStream.getRepresentation()->IsIncludedStream());
+
+    if (hasAdStream && m_timingStream != stream) // Do not "seek time" on "timing stream" already done above
+    {
+      if (!SeekAdStream(*stream, seekSecs))
+        continue;
+    }
+
+    if (!CheckReaderRunning(*stream))
+      continue;
+
+    streamReader->SetPTSDiff(ptsDiff);
+
+    if (!streamReader->TimeSeek(seekTimeCorrected, preceeding))
     {
-      bool reset{false};
-      // all streams must be started before seeking to ensure cross chapter seeks
-      // will seek to the correct location/segment
-      if (!streamReader->IsStarted())
-        StartReader(stream.get(), seekTimeCorrected, ptsDiff, preceeding, false);
-
-      streamReader->SetPTSDiff(ptsDiff);
-      
-      double seekSecs{static_cast<double>(seekTimeCorrected - ptsDiff) /
-                      STREAM_TIME_BASE};
-
-      // With FMP4 audio stream included to video stream you must not seek with AdaptiveStream
-      // segments are managed by AdaptiveStream of the video stream
-      // so CFragmentedSampleReader read data from the reader of the video stream
-      const bool noAdStream = streamReader->GetType() == ISampleReader::Type::FMP4 &&
-                              stream->m_adStream.getRepresentation()->IsIncludedStream();
-
-      if (noAdStream || stream->m_adStream.seek_time(seekSecs, preceeding, reset))
+      streamReader->Reset(true);
+    }
+    else
+    {
+      double destTime{static_cast<double>(PTSToElapsed(streamReader->PTS())) / STREAM_TIME_BASE};
+
+      // Note: TS sample reader with included streams have a dynamic streamId,
+      // so could be that will be printed on log stream id referred to audio or video,
+      // maybe this could be improved on sample reader to always start the seek from a video packet
+      LOG::Log(LOGINFO, "Seek time %0.1lf for stream: %i continues at %0.1lf (PTS: %llu)", seekTime,
+               streamReader->GetStreamId(), destTime, streamReader->PTS());
+
+      if (stream->m_info.GetStreamType() == INPUTSTREAM_TYPE_VIDEO)
       {
-        if (reset)
-          streamReader->Reset(false);
-        // advance reader to requested time
-        if (!streamReader->TimeSeek(seekTimeCorrected, preceeding))
-        {
-          streamReader->Reset(true);
-        }
-        else
-        {
-          double destTime{static_cast<double>(PTSToElapsed(streamReader->PTS())) /
-                          STREAM_TIME_BASE};
-          LOG::Log(LOGINFO,
-                   "Seek time %0.1lf for stream: %i (physical index %u) continues at %0.1lf "
-                   "(PTS: %llu)",
-                   seekTime, streamReader->GetStreamId(), stream->m_info.GetPhysicalIndex(),
-                   destTime, streamReader->PTS());
-          if (stream->m_info.GetStreamType() == INPUTSTREAM_TYPE_VIDEO)
-          {
-            seekTime = destTime;
-            seekTimeCorrected = streamReader->PTS();
-            preceeding = false;
-          }
-          ret = true;
-        }
+        seekTime = destTime;
+        seekTimeCorrected = streamReader->PTS();
+        preceeding = false;
       }
-      else
-        streamReader->Reset(true);
+      ret = true;
     }
   }
 

src/Session.h

@@ -131,16 +131,6 @@ class ATTR_DLL_LOCAL CSession : public adaptive::AdaptiveStreamObserver
    */
   uint64_t GetTimeshiftBufferStart();
 
-  /*! \brief Align adaptiveStream and start stream reader
-   *  \param stream The stream to start
-   *  \param seekTime The pts value to start the stream at
-   *  \param ptsDiff The pts difference to adjust seekTime by
-   *  \param preceeding True to ask reader to seek to preceeding sync point
-   *  \param timing True if this is the initial starting stream
-   */
-  void StartReader(
-      CStream* stream, uint64_t seekTime, int64_t ptsDiff, bool preceeding, bool timing);
-
   /*! \brief Check if the stream has changed, reset changed status
    *  \param bSet True to keep m_changed value true
    *  \return True if stream has changed

src/common/AdaptiveStream.cpp

@@ -271,17 +271,57 @@ void adaptive::AdaptiveStream::ResetSegment(const PLAYLIST::CSegment& segment)
   }
 }
 
-void adaptive::AdaptiveStream::ResetActiveBuffer()
+bool adaptive::AdaptiveStream::AlignActiveBufferToSegment(const PLAYLIST::CSegment& segment)
 {
-  thread_data_->StopDownloads();
+  if (m_segBuffers.ContainsSegment(segment))
   {
-    std::lock_guard<std::mutex> lckWorker(thread_data_->mutexWorker);
-    absolute_position_ = 0;
-    segment_read_pos_ = 0;
+    thread_data_->PauseDownloads();
+    // Check if there is a download in progress, if the segment being downloaded
+    // is older than the one requested, immediately stop the download
+    if (thread_data_->mutexWorker.try_lock())
+    {
+      thread_data_->mutexWorker.unlock();
+    }
+    else // You cant take the lock, there is a download in progress
+    {
+      // Wait to block read/write operations
+      std::unique_lock<std::mutex> lckrw(thread_data_->mutexRW);
+      thread_data_->cvRW.wait(lckrw);
+      // Note: doesnt matter if the download is finished right now and GetNextDownload return next buffer
+      SegmentBuffer* currDownload = m_segBuffers.GetNextDownload();
+      if (currDownload && currDownload->State() == BufferState::DOWNLOADING)
+      {
+        const uint64_t downloadStartPTS = currDownload->segment.startPTS_;
+        const uint64_t targetStartPTS = segment.startPTS_;
 
-    m_segBuffers.Reset();
+        if (downloadStartPTS < targetStartPTS)
+          thread_data_->StopDownloads();
+      }
+    }
+    {
+      std::lock_guard<std::mutex> lckWorker(thread_data_->mutexWorker);
+      while (!m_segBuffers.IsEmpty())
+      {
+        if (!segment.IsSame(m_segBuffers.Front().segment))
+          m_segBuffers.PopFront();
+        else
+          break;
+      }
+    }
+  }
+  else
+  {
+    thread_data_->StopDownloads();
+    {
+      std::lock_guard<std::mutex> lckWorker(thread_data_->mutexWorker);
+      m_segBuffers.Reset();
+    }
   }
+
+  absolute_position_ = 0;
+  segment_read_pos_ = 0;
   thread_data_->StartDownloads();
+  return !m_segBuffers.IsEmpty();
 }
 
 void adaptive::AdaptiveStream::worker()
@@ -1130,17 +1170,21 @@ void adaptive::AdaptiveStream::Disable()
   m_startEvent = EVENT_TYPE::STREAM_ENABLE;
 }
 
-void adaptive::AdaptiveStream::ResetCurrentSegment(const PLAYLIST::CSegment& newSegment)
+void adaptive::AdaptiveStream::UpdateCurrentSegment(const PLAYLIST::CSegment& newSegment)
 {
-  // EnsureSegment always loads the segment following the one specified as current, then sets the previous one
-  const CSegment* prevSeg = current_rep_->Timeline().GetPrevious(newSegment);
-  if (prevSeg)
-    current_rep_->current_segment_ = *prevSeg;
+  if (AlignActiveBufferToSegment(newSegment))
+  {
+    current_rep_->current_segment_ = m_segBuffers.Front().segment;
+  }
   else
-    current_rep_->current_segment_.reset();
-
-  // TODO: if new segment is already prefetched, don't ResetActiveBuffer;
-  ResetActiveBuffer();
+  {
+    // EnsureSegment always loads the segment following the one specified as current, then sets the previous one
+    const CSegment* prevSeg = current_rep_->Timeline().GetPrevious(newSegment);
+    if (prevSeg)
+      current_rep_->current_segment_ = *prevSeg;
+    else
+      current_rep_->current_segment_.reset();
+  }
 }
 
 int adaptive::AdaptiveStream::GetTrackType() const
@@ -1190,44 +1234,17 @@ bool adaptive::AdaptiveStream::seek_time(double seek_seconds, bool preceeding, b
   std::lock_guard<adaptive::AdaptiveTree::TreeUpdateThread> lckUpdTree(m_tree->GetTreeUpdMutex());
 
   const uint64_t pts = static_cast<uint64_t>(seek_seconds * current_rep_->GetTimescale());
-
   const CSegment* seekSeg = current_rep_->Timeline().FindByPTSOrNext(pts);
-  const std::optional<CSegment> oldSeg = current_rep_->current_segment_;
 
-  if (seekSeg)
-  {
-    if (oldSeg.has_value() && !seekSeg->IsSame(*oldSeg))
-    {
-      ResetCurrentSegment(*seekSeg);
-    }
-    else if (!preceeding)
-    {
-      // restart stream if it has 'finished', e.g in the case of subtitles
-      // where there may be a few or only one segment for the period and
-      // the stream is now in EOS state (all data already passed to Kodi)
-
-      //! @TODO: Twice downloaded segments, cause of video seek delay
-      //! Steps to reproduce:
-      //!   Play any kind of video, do a video seek
-      //!   on the log after "PosTime" callback print you can see twice downloaded segments
-      //!   a better way to debug is add a new log into AdaptiveStream::ensureSegment to the code related to Push new segments
-      //!   into segment buffers var and so print the segment numbers of the segments added to buffers.
-      //! the problem is also related to CSession::StartReader called by CSession::SeekTime,
-      //! In short the adaptive stream start to download segments seem just to know the "PTS diff"
-      //! and these downloads will be deleted just later without to be read, because CSession::SeekTime call these
-      //! method two times so by starting two times downloads.
-      //! This code has been introduced as part of subtitles fixes from https://github.com/xbmc/inputstream.adaptive/pull/1082
-      ResetCurrentSegment(*seekSeg);
-
-      absolute_position_ -= segment_read_pos_;
-      segment_read_pos_ = 0;
-    }
-    else
-      needReset = false;
-    return true;
-  }
+  if (!seekSeg)
+    return false;
 
-  return false;
+  UpdateCurrentSegment(*seekSeg);
+
+  if (preceeding)
+    needReset = false;
+
+  return true;
 }
 
 bool adaptive::AdaptiveStream::waitingForSegment() const
@@ -1371,6 +1388,11 @@ void adaptive::AdaptiveStream::THREADDATA::StopDownloads()
   m_state = ThState::STOPPED;
 }
 
+void adaptive::AdaptiveStream::THREADDATA::PauseDownloads()
+{
+  m_state = ThState::PAUSED;
+}
+
 void adaptive::AdaptiveStream::THREADDATA::StartDownloads()
 {
   m_state = ThState::RUNNING;