From 08656fa6e33739fc04c53104b81984659899166d Mon Sep 17 00:00:00 2001
From: Chris Tallon <chris@vomp.tv>
Date: Wed, 20 May 2020 20:01:56 +0100
Subject: [PATCH] OsdOpenVG: Tighten up sync between putOpenVGCommands and
 processOpenVGCommands

---
 osdopenvg.cc | 79 +++++++++++++++++-----------------------------------
 osdopenvg.h  | 18 ++++++------
 osdvector.cc |  2 +-
 3 files changed, 34 insertions(+), 65 deletions(-)

diff --git a/osdopenvg.cc b/osdopenvg.cc
index f867063..d21e78c 100644
--- a/osdopenvg.cc
+++ b/osdopenvg.cc
@@ -467,85 +467,56 @@ void OsdOpenVG::threadMethod()
   eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
 }
 
-unsigned int OsdOpenVG::putOpenVGCommand(OpenVGCommand& comm, bool wait)
+UINT OsdOpenVG::putOpenVGCommand(OpenVGCommand& comm, bool wait)
 {
-  taskmutex.lock();
-
-  if (wait)
-    comm.id = wait_id++;
-  else
-    comm.id = 0; // we are not waiting
+  if (wait) comm.response = new OpenVGResponse;
 
+  std::unique_lock<std::mutex> ul(taskmutex);     // locks
   vgcommands.push_back(comm);
-  taskmutex.unlock();
+  ul.unlock();
 
-  threadSignal();
+  threadSignal(); // signal renderLoop
 
   if (!wait) return 0;
 
-  while(1)
-  {
-    // Is there a response now?
-    taskmutex.lock();
-    for (auto& vgresponse : vgresponses)
-    {
-      if (vgresponse.id == comm.id)
-      {
-        UINT resp = vgresponse.result;
-        taskmutex.unlock();
-        return resp;
-      }
-    }
-    // No.
-    taskmutex.unlock();
+  ul.lock();
+  // Wait until done is true and we are signalled. If done is already true, this returns immediately
+  comm.response->doneCond.wait(ul, [&comm]{ return comm.response->done; });
 
-    std::unique_lock<std::mutex> ul(vgTaskSignalMutex);
-    vgTaskSignal.wait_for(ul, std::chrono::milliseconds(100));
-    ul.unlock();
-  }
+  // Now response->done = true, mutex is locked, we have been signalled
+
+  ul.unlock();
+  UINT toReturn = comm.response->result;
+  delete comm.response;
+  return toReturn;
 }
 
 bool OsdOpenVG::processOpenVGCommands()
 {
   bool worked = false;
-  taskmutex.lock();
-  vgmutex.lock();
 
+  taskmutex.lock();
   while (vgcommands.size() > 0)
   {
+    worked = true;
+
     OpenVGCommand comm = vgcommands.front();
     vgcommands.pop_front();
     taskmutex.unlock();
 
-    OpenVGResponse resp;
-    resp.result = handleOpenVGCommand(comm);
-    resp.id = comm.id;
-    taskmutex.lock();
-
-    if (comm.id)
-    {
-      vgresponses.push_back(resp);
-    }
-
-    taskmutex.unlock();
+    vgmutex.lock();
+    UINT vgResult = handleOpenVGCommand(comm);
     vgmutex.unlock();
 
-    /* Getting rid of Signal class. As with VideoOMX, just replicate what Signal did here
-     * and figure out if any of this can be simplified later. e.g. taskmutex sounds
-     * like it should be the mutex being used. 3 mutexes here???
-     */
-
-    vgTaskSignalMutex.lock();
-    vgTaskSignal.notify_one();
-    vgTaskSignalMutex.unlock();
-
     taskmutex.lock();
-    vgmutex.lock();
-    worked = true;
+    if (comm.response) // There is a response object, a thread is waiting
+    {
+      comm.response->result = vgResult;
+      comm.response->done = true;
+      comm.response->doneCond.notify_one();
+    }
   }
-
   taskmutex.unlock();
-  vgmutex.unlock();
 
   return worked;
 }
diff --git a/osdopenvg.h b/osdopenvg.h
index df91d52..87c074a 100644
--- a/osdopenvg.h
+++ b/osdopenvg.h
@@ -59,19 +59,21 @@ enum OpenVGTask
   OVGreadyEGLImage
 };
 
+struct OpenVGResponse
+{
+  bool done{};
+  UINT result;
+  std::condition_variable doneCond;
+};
+
 struct OpenVGCommand
 {
   enum OpenVGTask task;
   const void* data;
   const void* data2;
   unsigned int param1, param2, param3;
-  unsigned int id; //only set an id if you are waiting
-};
 
-struct OpenVGResponse
-{
-  unsigned int id;
-  unsigned int result;
+  struct OpenVGResponse* response{}; // If !NULL, a thread is waiting
 };
 
 class OsdOpenVG : public OsdVector, public Thread_TYPE
@@ -126,16 +128,12 @@ class OsdOpenVG : public OsdVector, public Thread_TYPE
 
     std::mutex vgmutex;
     std::mutex taskmutex;
-    std::mutex vgTaskSignalMutex;
-    std::condition_variable vgTaskSignal;
 
     std::deque<OpenVGCommand> vgcommands;
-    std::deque<OpenVGResponse> vgresponses;
     bool processOpenVGCommands();
     unsigned int putOpenVGCommand(OpenVGCommand& comm, bool wait);
     unsigned int handleOpenVGCommand(OpenVGCommand& command);
     //void purgeAllReferences();
-    unsigned int wait_id{1};
 
     FT_Library  ft_library;
     FT_Face     ft_face;
diff --git a/osdvector.cc b/osdvector.cc
index 453be13..5779637 100644
--- a/osdvector.cc
+++ b/osdvector.cc
@@ -337,7 +337,7 @@ void OsdVector::updateOrAddSurface(const SurfaceVector* surf, float x, float y,
 
 void OsdVector::removeSurface(const SurfaceVector* surf)
 {
-  std::lock_guard<std::mutex> lg(surfaces_mutex);
+  std::lock_guard<std::mutex> lg(surfaces_mutex);                      // FIXME - Can block here on shutdown if a timer is fired just as the wrong time
   for (auto i = surfaces.begin(); i != surfaces.end(); i++)
   {
     if (i->surface == surf)
-- 
2.39.5