Hi,
I would propose the following patch, or some equivalent interface that
would allow cThread::mutex to be used with some cCondVar in derived
classes:
diff --git a/thread.h b/thread.h
index 16c4bd75..cd1d98ab 100644
--- a/thread.h
+++ b/thread.h
@@ -83,7 +83,9 @@ private:
bool running;
pthread_t childTid;
tThreadId childThreadId;
+protected:
cMutex mutex;
+private:
char *description;
bool lowPriority;
static tThreadId mainThreadId;
Because cThread::mutx is declared private and there is no helper member
function, derived classes that wish to use condition variables have to
instantiate a separate cMutex for that. Here is an example from the
rpihddevice plugin that illustrates my point:
diff --git a/omx.c b/omx.c
--- a/omx.c
+++ b/omx.c
@@ -119,17 +119,17 @@ const char* cOmx::errStr(int err)
void cOmx::Action(void)
{
cTimeMs timer;
- m_mutex.Lock();
+ Lock();
while (Running())
{
while (m_portEvents.empty())
- if (m_portEventsAdded.TimedWait(m_mutex, 10))
+ if (!m_portEventsAdded.TimedWait(mutex, 10))
goto timeout;
{
const Event event = m_portEvents.front();
m_portEvents.pop();
- m_mutex.Unlock();
+ Unlock();
switch (event.event)
{
Actually, there is a bug above: the condition for the TimedWait() call
was inverted.
This code illustrates another limitation: There is no way to pass an
absolute time to cCondVar::TimedWait(). On each call, a relative wake-up
time (milliseconds from the current time) will be converted into an
absolute time. If there was a way, we would be able to remove the
"cTimeMs timer" and some related system calls, and have this loop both
wake up every 100 milliseconds, and process events as soon as they
arrive. Here is the VDR part of the patch:
diff --git a/thread.c b/thread.c
index 93eb8c0d..3dcb44d4 100644
--- a/thread.c
+++ b/thread.c
@@ -36,7 +36,7 @@
#define dbglocking(a...)
#endif
-static bool GetAbsTime(struct timespec *Abstime, int MillisecondsFromNow)
+bool cCondVar::GetAbsTime(struct timespec *Abstime, int MillisecondsFromNow)
{
struct timeval now;
if (gettimeofday(&now, NULL) == 0) { // get current time
@@ -81,7 +81,7 @@ bool cCondWait::Wait(int TimeoutMs)
if (!signaled) {
if (TimeoutMs) {
struct timespec abstime;
- if (GetAbsTime(&abstime, TimeoutMs)) {
+ if (cCondVar::GetAbsTime(&abstime, TimeoutMs)) {
while (!signaled) {
if (pthread_cond_timedwait(&cond, &mutex, &abstime) == ETIMEDOUT)
break;
@@ -129,20 +129,27 @@ void cCondVar::Wait(cMutex &Mutex)
}
}
+bool cCondVar::TimedWait(cMutex &Mutex, const timespec &abstime)
+{
+ int err = 0;
+
+ if (int locked = Mutex.locked) {
+ Mutex.locked = 0; // have to clear the locked count here, as pthread_cond_timedwait
+ // does an implicit unlock of the mutex.
+ err = pthread_cond_timedwait(&cond, &Mutex.mutex, &abstime);
+ Mutex.locked = locked;
+ }
+ return err != ETIMEDOUT;
+}
+
bool cCondVar::TimedWait(cMutex &Mutex, int TimeoutMs)
{
bool r = true; // true = condition signaled, false = timeout
if (Mutex.locked) {
struct timespec abstime;
- if (GetAbsTime(&abstime, TimeoutMs)) {
- int locked = Mutex.locked;
- Mutex.locked = 0; // have to clear the locked count here, as pthread_cond_timedwait
- // does an implicit unlock of the mutex.
- if (pthread_cond_timedwait(&cond, &Mutex.mutex, &abstime) == ETIMEDOUT)
- r = false;
- Mutex.locked = locked;
- }
+ if (GetAbsTime(&abstime, TimeoutMs))
+ r = TimedWait(Mutex, abstime);
}
return r;
}
@@ -174,7 +181,7 @@ bool cRwLock::Lock(bool Write, int TimeoutMs)
int Result = 0;
struct timespec abstime;
if (TimeoutMs) {
- if (!GetAbsTime(&abstime, TimeoutMs))
+ if (!cCondVar::GetAbsTime(&abstime, TimeoutMs))
TimeoutMs = 0;
}
if (Write) {
diff --git a/thread.h b/thread.h
index 16c4bd75..04bb4cc5 100644
--- a/thread.h
+++ b/thread.h
@@ -49,7 +49,9 @@ public:
~cCondVar();
void Wait(cMutex &Mutex);
bool TimedWait(cMutex &Mutex, int TimeoutMs);
+ bool TimedWait(cMutex &Mutex, const timespec &abstime);
void Broadcast(void);
+ static bool GetAbsTime(struct timespec *Abstime, int MillisecondsFromNow);
};
class cRwLock {
I did not complete the change to rpihddevice cOmx::Action() yet. We may
be forced to retain two mutexes after all, because OMX_EmptyThisBuffer()
as well as one of the functions called from cOmx::StopAudio() would hang
indefinitely, while holding the cOmx::mutex, blocking the cOmx::Action()
thread. According to
https://forums.raspberrypi.com/viewtopic.php?t=313170 the interface is
largely deprecated. I will try to figure out something.
Best regards,
Marko
