VideoPlayer: vaapi - do not destruct postproc if buffers are still in use

2 files changed, 138 insertions, 85 deletions

diff --git a/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp b/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp
index 8006084f63..a19cdf404e 100644
--- a/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp
+++ b/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp
@@ -1832,8 +1832,18 @@ bool COutput::Uninit()
 {
   ProcessSyncPicture();
   ReleaseBufferPool();
-  delete m_pp;
-  m_pp = NULL;
+  if (m_pp)
+  {
+    std::shared_ptr<CPostproc> pp(m_pp);
+    m_discardedPostprocs.push_back(pp);
+    m_pp->Discard(this, &COutput::ReadyForDisposal);
+    m_pp = nullptr;
+  }
+
+  if (!m_discardedPostprocs.empty())
+  {
+    CLog::Log(LOGERROR, "VAAPI::COutput::Uninit - not all CPostprcs released");
+  }
   return true;
 }
 
@@ -1939,9 +1949,10 @@ void COutput::InitCycle()
 
     if (m_pp && !m_pp->UpdateDeintMethod(method))
     {
-      delete m_pp;
-      m_pp = NULL;
-      DropVppProcessedPictures();
+      std::shared_ptr<CPostproc> pp(m_pp);
+      m_discardedPostprocs.push_back(pp);
+      m_pp->Discard(this, &COutput::ReadyForDisposal);
+      m_pp = nullptr;
       m_config.processInfo->SetVideoDeintMethod("unknown");
     }
     if (!m_pp)
@@ -1985,9 +1996,10 @@ void COutput::InitCycle()
     method = VS_INTERLACEMETHOD_NONE;
     if (m_pp && !m_pp->UpdateDeintMethod(method))
     {
-      delete m_pp;
-      m_pp = NULL;
-      DropVppProcessedPictures();
+      std::shared_ptr<CPostproc> pp(m_pp);
+      m_discardedPostprocs.push_back(pp);
+      m_pp->Discard(this, &COutput::ReadyForDisposal);
+      m_pp = nullptr;
     }
     if (!m_pp)
     {
@@ -2025,8 +2037,14 @@ CVaapiRenderPicture* COutput::ProcessPicture(CVaapiProcessedPicture &pic)
   retPic = m_bufferPool->GetVaapi();
   retPic->DVDPic.SetParams(pic.DVDPic);
 
-  if (pic.source == CVaapiProcessedPicture::SKIP_SRC ||
-      pic.source == CVaapiProcessedPicture::VPP_SRC)
+  if (!pic.source)
+  {
+    CLog::Log(LOGERROR, "VAAPI::ProcessPicture - pic has no source");
+    retPic->Release();
+    return nullptr;
+  }
+
+  if (pic.source->UseVideoSurface())
   {
     vaSyncSurface(m_config.dpy, pic.videoSurface);
     pic.id = m_bufferPool->procPicId++;
@@ -2034,17 +2052,12 @@ CVaapiRenderPicture* COutput::ProcessPicture(CVaapiProcessedPicture &pic)
     retPic->procPic = pic;
     retPic->vadsp = m_config.dpy;
   }
-  else if (pic.source == CVaapiProcessedPicture::FFMPEG_SRC)
+  else
   {
     av_frame_move_ref(retPic->avFrame, pic.frame);
-    av_frame_free(&pic.frame);
+    pic.source->ClearRef(pic);
     retPic->procPic.videoSurface = VA_INVALID_ID;
   }
-  else
-  {
-    retPic->Release();
-    return nullptr;
-  }
 
   retPic->DVDPic.dts = DVD_NOPTS_VALUE;
   retPic->DVDPic.iWidth = m_config.vidWidth;
@@ -2057,50 +2070,12 @@ CVaapiRenderPicture* COutput::ProcessPicture(CVaapiProcessedPicture &pic)
 
 void COutput::ReleaseProcessedPicture(CVaapiProcessedPicture &pic)
 {
-  if (pic.source == CVaapiProcessedPicture::VPP_SRC && m_pp)
+  if (!pic.source)
   {
-    CVppPostproc *pp = dynamic_cast<CVppPostproc*>(m_pp);
-    if (pp)
-    {
-      pp->ClearRef(pic.videoSurface);
-    }
-  }
-  else if (pic.source == CVaapiProcessedPicture::SKIP_SRC)
-  {
-    m_config.videoSurfaces->ClearRender(pic.videoSurface);
-  }
-  else if (pic.source == CVaapiProcessedPicture::FFMPEG_SRC)
-  {
-    av_frame_free(&pic.frame);
-  }
-}
-
-void COutput::DropVppProcessedPictures()
-{
-  auto it = m_bufferPool->processedPics.begin();
-  while (it != m_bufferPool->processedPics.end())
-  {
-    if (it->source == CVaapiProcessedPicture::VPP_SRC)
-    {
-      it = m_bufferPool->processedPics.erase(it);
-      m_config.stats->DecProcessed();
-    }
-    else
-      ++it;
-  }
-
-  it = m_bufferPool->processedPicsAway.begin();
-  while (it != m_bufferPool->processedPicsAway.end())
-  {
-    if (it->source == CVaapiProcessedPicture::VPP_SRC)
-    {
-      it = m_bufferPool->processedPicsAway.erase(it);
-    }
-    else
-      ++it;
+    return;
   }
-
-  m_controlPort.SendInMessage(COutputControlProtocol::STATS);
+  pic.source->ClearRef(pic);
+  pic.source = nullptr;
 }
 
 void COutput::QueueReturnPicture(CVaapiRenderPicture *pic)
@@ -2167,6 +2142,18 @@ void COutput::ReleaseBufferPool(bool precleanup)
   m_bufferPool->processedPics.clear();
 }
 
+void COutput::ReadyForDisposal(CPostproc *pp)
+{
+  for (auto it = m_discardedPostprocs.begin(); it != m_discardedPostprocs.end(); ++it)
+  {
+    if ((*it).get() == pp)
+    {
+      m_discardedPostprocs.erase(it);
+      break;
+    }
+  }
+}
+
 bool COutput::CheckSuccess(VAStatus status)
 {
   if (status != VA_STATUS_SUCCESS)
@@ -2202,11 +2189,12 @@ bool CSkipPostproc::AddPicture(CVaapiDecodedPicture &inPic)
 
 bool CSkipPostproc::Filter(CVaapiProcessedPicture &outPic)
 {
-  if (m_step>0)
+  if (m_step > 0)
     return false;
   outPic.DVDPic.SetParams(m_pic.DVDPic);
   outPic.videoSurface = m_pic.videoSurface;
-  outPic.source = CVaapiProcessedPicture::SKIP_SRC;
+  m_refsToSurfaces++;
+  outPic.source = this;
   outPic.DVDPic.iFlags &= ~(DVP_FLAG_TOP_FIELD_FIRST |
                             DVP_FLAG_REPEAT_TOP_FIELD |
                             DVP_FLAG_INTERLACED);
@@ -2214,9 +2202,13 @@ bool CSkipPostproc::Filter(CVaapiProcessedPicture &outPic)
   return true;
 }
 
-void CSkipPostproc::ClearRef(VASurfaceID surf)
+void CSkipPostproc::ClearRef(CVaapiProcessedPicture &pic)
 {
+  m_config.videoSurfaces->ClearRender(pic.videoSurface);
+  m_refsToSurfaces--;
 
+  if (m_pOut && m_refsToSurfaces <= 0)
+    (m_pOut->*m_cbDispose)(this);
 }
 
 void CSkipPostproc::Flush()
@@ -2237,6 +2229,19 @@ bool CSkipPostproc::DoesSync()
   return false;
 }
 
+bool CSkipPostproc::UseVideoSurface()
+{
+  return true;
+}
+
+void CSkipPostproc::Discard(COutput *output, ReadyToDispose cb)
+{
+  m_pOut = output;
+  m_cbDispose = cb;
+  if (m_refsToSurfaces <= 0)
+    (m_pOut->*m_cbDispose)(this);
+}
+
 //-----------------------------------------------------------------------------
 // VPP Postprocessing
 //-----------------------------------------------------------------------------
@@ -2651,7 +2656,7 @@ bool CVppPostproc::Filter(CVaapiProcessedPicture &outPic)
 
   m_step++;
   outPic.videoSurface = m_videoSurfaces.GetFree(surf);
-  outPic.source = CVaapiProcessedPicture::VPP_SRC;
+  outPic.source = this;
   outPic.DVDPic.iFlags &= ~(DVP_FLAG_TOP_FIELD_FIRST |
                             DVP_FLAG_REPEAT_TOP_FIELD |
                             DVP_FLAG_INTERLACED);
@@ -2677,9 +2682,12 @@ void CVppPostproc::Advance()
   }
 }
 
-void CVppPostproc::ClearRef(VASurfaceID surf)
+void CVppPostproc::ClearRef(CVaapiProcessedPicture &pic)
 {
-  m_videoSurfaces.ClearReference(surf);
+  m_videoSurfaces.ClearReference(pic.videoSurface);
+
+  if (m_pOut && !m_videoSurfaces.HasRefs())
+    (m_pOut->*m_cbDispose)(this);
 }
 
 void CVppPostproc::Flush()
@@ -2722,6 +2730,19 @@ bool CVppPostproc::WantsPic()
   return false;
 }
 
+bool CVppPostproc::UseVideoSurface()
+{
+  return true;
+}
+
+void CVppPostproc::Discard(COutput *output, ReadyToDispose cb)
+{
+  m_pOut = output;
+  m_cbDispose = cb;
+  if (!m_videoSurfaces.HasRefs())
+    (m_pOut->*m_cbDispose)(this);
+}
+
 bool CVppPostproc::CheckSuccess(VAStatus status)
 {
   if (status != VA_STATUS_SUCCESS)
@@ -2812,7 +2833,7 @@ bool CFFmpegPostproc::Init(EINTERLACEMETHOD method)
 {
   if (!(m_pFilterGraph = avfilter_graph_alloc()))
   {
-    CLog::Log(LOGERROR, "CFFmpegPostproc::Init - unable to alloc filter graph");
+    CLog::Log(LOGERROR, "VAAPI::CFFmpegPostproc::Init - unable to alloc filter graph");
     return false;
   }
 
@@ -2830,7 +2851,7 @@ bool CFFmpegPostproc::Init(EINTERLACEMETHOD method)
 
   if (avfilter_graph_create_filter(&m_pFilterIn, srcFilter, "src", args.c_str(), NULL, m_pFilterGraph) < 0)
   {
-    CLog::Log(LOGERROR, "CFFmpegPostproc::Init - avfilter_graph_create_filter: src");
+    CLog::Log(LOGERROR, "VAAPI::CFFmpegPostproc::Init - avfilter_graph_create_filter: src");
     return false;
   }
 
@@ -2843,7 +2864,7 @@ bool CFFmpegPostproc::Init(EINTERLACEMETHOD method)
   enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_NV12, AV_PIX_FMT_NONE };
   if (av_opt_set_int_list(m_pFilterOut, "pix_fmts", pix_fmts,  AV_PIX_FMT_NONE, AV_OPT_SEARCH_CHILDREN) < 0)
   {
-    CLog::Log(LOGERROR, "CFFmpegPostproc::Init  - failed settings pix formats");
+    CLog::Log(LOGERROR, "VAAPI::CFFmpegPostproc::Init  - failed settings pix formats");
     return false;
   }
 
@@ -2868,7 +2889,7 @@ bool CFFmpegPostproc::Init(EINTERLACEMETHOD method)
 
     if (avfilter_graph_parse_ptr(m_pFilterGraph, filter.c_str(), &inputs, &outputs, NULL) < 0)
     {
-      CLog::Log(LOGERROR, "CFFmpegPostproc::Init  - avfilter_graph_parse");
+      CLog::Log(LOGERROR, "VAAPI::CFFmpegPostproc::Init  - avfilter_graph_parse");
       avfilter_inout_free(&outputs);
       avfilter_inout_free(&inputs);
       return false;
@@ -2879,14 +2900,14 @@ bool CFFmpegPostproc::Init(EINTERLACEMETHOD method)
 
     if (avfilter_graph_config(m_pFilterGraph, NULL) < 0)
     {
-      CLog::Log(LOGERROR, "CFFmpegPostproc::Init  - avfilter_graph_config");
+      CLog::Log(LOGERROR, "VAAPI::CFFmpegPostproc::Init  - avfilter_graph_config");
       return false;
     }
   }
   else if (method == VS_INTERLACEMETHOD_RENDER_BOB ||
            method == VS_INTERLACEMETHOD_NONE)
   {
-    CLog::Log(LOGDEBUG, LOGVIDEO, "CFFmpegPostproc::Init  - skip deinterlacing");
+    CLog::Log(LOGDEBUG, LOGVIDEO, "VAAPI::CFFmpegPostproc::Init  - skip deinterlacing");
     avfilter_inout_free(&outputs);
     avfilter_inout_free(&inputs);
   }
@@ -3005,7 +3026,7 @@ bool CFFmpegPostproc::Filter(CVaapiProcessedPicture &outPic)
   else if (m_diMethod == VS_INTERLACEMETHOD_RENDER_BOB ||
            m_diMethod == VS_INTERLACEMETHOD_NONE)
   {
-    if (m_step>0)
+    if (m_step > 0)
       return false;
   }
 
@@ -3013,7 +3034,8 @@ bool CFFmpegPostproc::Filter(CVaapiProcessedPicture &outPic)
   outPic.frame = av_frame_clone(m_pFilterFrameOut);
   av_frame_unref(m_pFilterFrameOut);
 
-  outPic.source = CVaapiProcessedPicture::FFMPEG_SRC;
+  outPic.source = this;
+  m_refsToPics++;
 
   int64_t bpts = av_frame_get_best_effort_timestamp(outPic.frame);
   if(bpts != AV_NOPTS_VALUE)
@@ -3033,9 +3055,13 @@ bool CFFmpegPostproc::Filter(CVaapiProcessedPicture &outPic)
   return true;
 }
 
-void CFFmpegPostproc::ClearRef(VASurfaceID surf)
+void CFFmpegPostproc::ClearRef(CVaapiProcessedPicture &pic)
 {
+  av_frame_free(&pic.frame);
+  m_refsToPics--;
 
+  if (m_pOut && m_refsToPics <= 0)
+    (m_pOut->*m_cbDispose)(this);
 }
 
 void CFFmpegPostproc::Close()
@@ -3077,6 +3103,19 @@ bool CFFmpegPostproc::DoesSync()
   return true;
 }
 
+bool CFFmpegPostproc::UseVideoSurface()
+{
+  return false;
+}
+
+void CFFmpegPostproc::Discard(COutput *output, ReadyToDispose cb)
+{
+  m_pOut = output;
+  m_cbDispose = cb;
+  if (m_refsToPics <= 0)
+    (m_pOut->*m_cbDispose)(this);
+}
+
 bool CFFmpegPostproc::CheckSuccess(VAStatus status)
 {
   if (status != VA_STATUS_SUCCESS)
diff --git a/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.h b/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.h
index b753f98d94..45c4d1bc5c 100644
--- a/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.h
+++ b/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.h
@@ -146,6 +146,7 @@ struct CVaapiDecodedPicture
 /**
  * Frame after having been processed by vpp
  */
+class CPostproc;
 struct CVaapiProcessedPicture
 {
   CVaapiProcessedPicture() = default;
@@ -168,12 +169,7 @@ struct CVaapiProcessedPicture
   VASurfaceID videoSurface;
   AVFrame *frame;
   int id;
-  enum
-  {
-    VPP_SRC,
-    FFMPEG_SRC,
-    SKIP_SRC
-  }source;
+  CPostproc *source = nullptr;
   bool crop;
 };
 
@@ -270,13 +266,13 @@ protected:
   void ProcessReturnProcPicture(int id);
   void ProcessSyncPicture();
   void ReleaseProcessedPicture(CVaapiProcessedPicture &pic);
-  void DropVppProcessedPictures();
   bool Init();
   bool Uninit();
   void Flush();
   void EnsureBufferPool();
   void ReleaseBufferPool(bool precleanup = false);
   bool CheckSuccess(VAStatus status);
+  void ReadyForDisposal(CPostproc *pp);
   CEvent m_outMsgEvent;
   CEvent *m_inMsgEvent;
   int m_state;
@@ -290,6 +286,7 @@ protected:
   std::shared_ptr<CVaapiBufferPool> m_bufferPool;
   CVaapiDecodedPicture m_currentPicture;
   CPostproc *m_pp;
+  std::list<std::shared_ptr<CPostproc>> m_discardedPostprocs;
   SDiMethods m_diMethods;
 };
 
@@ -446,6 +443,7 @@ protected:
 /**
  *  Base class
  */
+typedef void (COutput::*ReadyToDispose)(CPostproc *pool);
 class CPostproc
 {
 public:
@@ -454,11 +452,13 @@ public:
   virtual bool Init(EINTERLACEMETHOD method) = 0;
   virtual bool AddPicture(CVaapiDecodedPicture &inPic) = 0;
   virtual bool Filter(CVaapiProcessedPicture &outPic) = 0;
-  virtual void ClearRef(VASurfaceID surf) = 0;
+  virtual void ClearRef(CVaapiProcessedPicture &pic) = 0;
   virtual void Flush() = 0;
   virtual bool UpdateDeintMethod(EINTERLACEMETHOD method) = 0;
   virtual bool DoesSync() = 0;
   virtual bool WantsPic() {return true;}
+  virtual bool UseVideoSurface() = 0;
+  virtual void Discard(COutput *output, ReadyToDispose cb) { (output->*cb)(this); };
 protected:
   CVaapiConfig m_config;
   int m_step;
@@ -474,12 +474,17 @@ public:
   bool Init(EINTERLACEMETHOD method) override;
   bool AddPicture(CVaapiDecodedPicture &inPic) override;
   bool Filter(CVaapiProcessedPicture &outPic) override;
-  void ClearRef(VASurfaceID surf) override;
+  void ClearRef(CVaapiProcessedPicture &pic) override;
   void Flush() override;
   bool UpdateDeintMethod(EINTERLACEMETHOD method) override;
   bool DoesSync() override;
+  bool UseVideoSurface() override;
+  void Discard(COutput *output, ReadyToDispose cb) override;
 protected:
   CVaapiDecodedPicture m_pic;
+  ReadyToDispose m_cbDispose;
+  COutput *m_pOut;
+  int m_refsToSurfaces = 0;
 };
 
 /**
@@ -494,11 +499,13 @@ public:
   bool Init(EINTERLACEMETHOD method) override;
   bool AddPicture(CVaapiDecodedPicture &inPic) override;
   bool Filter(CVaapiProcessedPicture &outPic) override;
-  void ClearRef(VASurfaceID surf) override;
+  void ClearRef(CVaapiProcessedPicture &pic) override;
   void Flush() override;
   bool UpdateDeintMethod(EINTERLACEMETHOD method) override;
   bool DoesSync() override;
   bool WantsPic() override;
+  bool UseVideoSurface() override;
+  void Discard(COutput *output, ReadyToDispose cb) override;
 protected:
   bool CheckSuccess(VAStatus status);
   void Dispose();
@@ -512,6 +519,8 @@ protected:
   int m_currentIdx;
   int m_frameCount;
   EINTERLACEMETHOD m_vppMethod;
+  ReadyToDispose m_cbDispose;
+  COutput *m_pOut = nullptr;
 };
 
 /**
@@ -526,10 +535,12 @@ public:
   bool Init(EINTERLACEMETHOD method) override;
   bool AddPicture(CVaapiDecodedPicture &inPic) override;
   bool Filter(CVaapiProcessedPicture &outPic) override;
-  void ClearRef(VASurfaceID surf) override;
+  void ClearRef(CVaapiProcessedPicture &pic) override;
   void Flush() override;
   bool UpdateDeintMethod(EINTERLACEMETHOD method) override;
   bool DoesSync() override;
+  bool UseVideoSurface() override;
+  void Discard(COutput *output, ReadyToDispose cb) override;
 protected:
   bool CheckSuccess(VAStatus status);
   void Close();
@@ -544,6 +555,9 @@ protected:
   VideoPicture m_DVDPic;
   double m_frametime;
   double m_lastOutPts;
+  ReadyToDispose m_cbDispose;
+  COutput *m_pOut;
+  int m_refsToPics = 0;
 };
 
 }