Repository: xbmc/xbmc
File: xbmc/cores/VideoPlayer/DVDOverlayContainer.cpp (C++)

Code before:
                       double pts,
                       std::set<double>& forcedStartTimes)
{
  // Remove non-forced overlays that have expired
  if (!overlay->bForced && overlay->iPTSStopTime <= pts && overlay->iPTSStopTime != 0.0)
    return true;

  // Remove forced overlays replaced by newer ones
  if (overlay->bForced)
  {
    if (!forcedStartTimes.empty() &&
        (forcedStartTimes.size() > 1 || *forcedStartTimes.begin() != overlay->iPTSStartTime))
      return true;

    if (overlay->iPTSStartTime < pts)
      forcedStartTimes.insert(overlay->iPTSStartTime);
  }
  return false;
}
} // namespace

void CDVDOverlayContainer::CleanUp(double pts)
{
  std::unique_lock lock(*this);

  std::set<double> forcedStartTimes;
  for (auto it = m_overlays.end(); it != m_overlays.begin();)
  {
    --it;
    if (IsOverlayFinished(*it, pts, forcedStartTimes))
      it = m_overlays.erase(it);
  }
}

void CDVDOverlayContainer::Flush()
{
  std::unique_lock lock(*this);

  // Flush only the overlays marked as flushable
  std::erase_if(m_overlays, [](const std::shared_ptr<CDVDOverlay>& ov)
                { return ov->IsOverlayContainerFlushable(); });
}

void CDVDOverlayContainer::Clear()
{
  std::unique_lock lock(*this);
  m_overlays.clear();
}

size_t CDVDOverlayContainer::GetSize() const
{

Review comment (performance):
This is not O(n) as claimed in the PR but O(n log n). Not sure how large the set is going to be but if it does have a lot of elements `std::unordered_set` can be worth it, for small sets though it is often slower.

Code after:
void CDVDOverlayContainer::CleanUp(double pts)
{
  std::unique_lock lock(*this);

  auto it = m_overlays.begin();
  while (it != m_overlays.end())
  {
    const std::shared_ptr<CDVDOverlay>& pOverlay = *it;

    // never delete forced overlays, they are used in menu's
    // clear takes care of removing them
    // also if stoptime = 0, it means the next subtitles will use its starttime as the stoptime
    // which means we cannot delete overlays with stoptime 0
    if (!pOverlay->bForced && pOverlay->iPTSStopTime <= pts && pOverlay->iPTSStopTime != 0)
    {
      //CLog::Log(LOGDEBUG,"CDVDOverlay::CleanUp, removing {}", (int)(pts / 1000));
      //CLog::Log(LOGDEBUG,"CDVDOverlay::CleanUp, remove, start : {}, stop : {}", (int)(pOverlay->iPTSStartTime / 1000), (int)(pOverlay->iPTSStopTime / 1000));
      it = Remove(it);
      continue;
    }
    else if (pOverlay->bForced)
    {
      //Check for newer replacements
      auto it2 = it;
      bool bNewer = false;
      while (!bNewer && ++it2 != m_overlays.end())
      {
        const std::shared_ptr<CDVDOverlay>& pOverlay2 = *it2;
        // There can be multiple overlays queued at same start point.
        // Skip them to find a new start point.
        if (pOverlay2->bForced && pOverlay2->iPTSStartTime <= pts)
          bNewer = true;
      }

      if (bNewer)
      {
        it = Remove(it);
        continue;
      }
    }
    ++it;
  }

}

void CDVDOverlayContainer::Flush()
{
  std::unique_lock lock(*this);

  // Flush only the overlays marked as flushable
  m_overlays.erase(std::remove_if(m_overlays.begin(), m_overlays.end(),