You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2023/01/20 14:47:32 UTC
[nuttx-apps] branch master updated (44d064233 -> 0bf87e789)
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git
from 44d064233 Add *.su to .gitignore
new 0e06cfeae system/media: replace sem lock with pthread_mutex
new 0bf87e789 system/media: fix race condion on pthread id
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
include/system/nxlooper.h | 33 ++++----
include/system/nxplayer.h | 35 ++++----
include/system/nxrecorder.h | 21 +++--
system/nxcamera/nxcamera.c | 56 ++++++++-----
system/nxlooper/nxlooper.c | 115 +++++++++++---------------
system/nxplayer/nxplayer.c | 179 ++++++++++++----------------------------
system/nxplayer/nxplayer_main.c | 2 -
system/nxrecorder/nxrecorder.c | 107 ++++++++++--------------
8 files changed, 222 insertions(+), 326 deletions(-)
[nuttx-apps] 01/02: system/media: replace sem lock with pthread_mutex
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git
commit 0e06cfeaec3341d87f907d75776d0ec7872f26f2
Author: ligd <li...@xiaomi.com>
AuthorDate: Tue Jan 3 19:28:25 2023 +0800
system/media: replace sem lock with pthread_mutex
Signed-off-by: ligd <li...@xiaomi.com>
---
include/system/nxlooper.h | 33 +++++-----
include/system/nxplayer.h | 35 +++++------
include/system/nxrecorder.h | 21 +++----
system/nxlooper/nxlooper.c | 69 +++++----------------
system/nxplayer/nxplayer.c | 133 +++++++---------------------------------
system/nxplayer/nxplayer_main.c | 2 -
system/nxrecorder/nxrecorder.c | 61 ++++--------------
7 files changed, 93 insertions(+), 261 deletions(-)
diff --git a/include/system/nxlooper.h b/include/system/nxlooper.h
index 51f45c758..becbea97d 100644
--- a/include/system/nxlooper.h
+++ b/include/system/nxlooper.h
@@ -29,7 +29,6 @@
#include <mqueue.h>
#include <pthread.h>
-#include <semaphore.h>
/****************************************************************************
* Pre-processor Definitions
@@ -43,30 +42,30 @@
struct nxlooper_s
{
- int loopstate; /* Current looper test state */
- int recorddev_fd; /* File descriptor of active
- * record device */
- char recorddev[CONFIG_NAME_MAX]; /* Preferred record device */
- int playdev_fd; /* File descriptor of active
- * play device */
- char playdev[CONFIG_NAME_MAX]; /* Preferred loopback device */
- int crefs; /* Number of references */
- sem_t sem; /* Thread sync semaphore */
- char mqname[16]; /* Name of play message queue */
- mqd_t mq; /* Message queue for the
- * loopthread */
- pthread_t loop_id; /* Thread ID of the loopthread */
+ int loopstate; /* Current looper test state */
+ int recorddev_fd; /* File descriptor of active
+ * record device */
+ char recorddev[CONFIG_NAME_MAX]; /* Preferred record device */
+ int playdev_fd; /* File descriptor of active
+ * play device */
+ char playdev[CONFIG_NAME_MAX]; /* Preferred loopback device */
+ int crefs; /* Number of references */
+ pthread_mutex_t mutex; /* Thread sync mutex */
+ char mqname[16]; /* Name of play message queue */
+ mqd_t mq; /* Message queue for the
+ * loopthread */
+ pthread_t loop_id; /* Thread ID of the loopthread */
#ifdef CONFIG_AUDIO_MULTI_SESSION
- FAR void *pplayses; /* Session assignment from device */
+ FAR void *pplayses; /* Session assignment from device */
#endif
#ifdef CONFIG_AUDIO_MULTI_SESSION
- FAR void *precordses; /* Session assignment from device */
+ FAR void *precordses; /* Session assignment from device */
#endif
#ifndef CONFIG_AUDIO_EXCLUDE_VOLUME
- uint16_t volume; /* Volume as a whole percentage (0-100) */
+ uint16_t volume; /* Volume as a whole percentage (0-100) */
#endif
};
diff --git a/include/system/nxplayer.h b/include/system/nxplayer.h
index cc8e01879..ba2c65630 100644
--- a/include/system/nxplayer.h
+++ b/include/system/nxplayer.h
@@ -29,7 +29,6 @@
#include <mqueue.h>
#include <pthread.h>
-#include <semaphore.h>
/****************************************************************************
* Pre-processor Definitions
@@ -51,34 +50,34 @@ struct nxplayer_dec_ops_s
struct nxplayer_s
{
- int state; /* Current player state */
- int dev_fd; /* File descriptor of active device */
- mqd_t mq; /* Message queue for the playthread */
- char mqname[16]; /* Name of our message queue */
- pthread_t play_id; /* Thread ID of the playthread */
- int crefs; /* Number of references to the player */
- sem_t sem; /* Thread sync semaphore */
- int fd; /* File descriptor of open file */
+ int state; /* Current player state */
+ int dev_fd; /* File descriptor of active device */
+ mqd_t mq; /* Message queue for the playthread */
+ char mqname[16]; /* Name of our message queue */
+ pthread_t play_id; /* Thread ID of the playthread */
+ int crefs; /* Number of references to the player */
+ pthread_mutex_t mutex; /* Thread sync mutex */
+ int fd; /* File descriptor of open file */
#ifdef CONFIG_NXPLAYER_INCLUDE_PREFERRED_DEVICE
- char prefdevice[CONFIG_NAME_MAX]; /* Preferred audio device */
- int prefformat; /* Formats supported by preferred device */
- int preftype; /* Types supported by preferred device */
+ char prefdevice[CONFIG_NAME_MAX]; /* Preferred audio device */
+ int prefformat; /* Formats supported by preferred device */
+ int preftype; /* Types supported by preferred device */
#endif
#ifdef CONFIG_NXPLAYER_INCLUDE_MEDIADIR
- char mediadir[CONFIG_NAME_MAX]; /* Root media directory where media is located */
+ char mediadir[CONFIG_NAME_MAX]; /* Root media directory where media is located */
#endif
#ifdef CONFIG_AUDIO_MULTI_SESSION
- FAR void *session; /* Session assignment from device */
+ FAR void *session; /* Session assignment from device */
#endif
#ifndef CONFIG_AUDIO_EXCLUDE_VOLUME
- uint16_t volume; /* Volume as a whole percentage (0-100) */
+ uint16_t volume; /* Volume as a whole percentage (0-100) */
#ifndef CONFIG_AUDIO_EXCLUDE_BALANCE
- uint16_t balance; /* Balance as a whole % (0=left off, 100=right off) */
+ uint16_t balance; /* Balance as a whole % (0=left off, 100=right off) */
#endif
#endif
#ifndef CONFIG_AUDIO_EXCLUDE_TONE
- uint16_t treble; /* Treble as a whole % */
- uint16_t bass; /* Bass as a whole % */
+ uint16_t treble; /* Treble as a whole % */
+ uint16_t bass; /* Bass as a whole % */
#endif
FAR const struct nxplayer_dec_ops_s *ops;
diff --git a/include/system/nxrecorder.h b/include/system/nxrecorder.h
index b632c4a2d..129a23170 100644
--- a/include/system/nxrecorder.h
+++ b/include/system/nxrecorder.h
@@ -29,7 +29,6 @@
#include <mqueue.h>
#include <pthread.h>
-#include <semaphore.h>
/****************************************************************************
* Pre-processor Definitions
@@ -43,17 +42,17 @@
struct nxrecorder_s
{
- int state; /* Current recorder state */
- int dev_fd; /* File descriptor of active device */
- mqd_t mq; /* Message queue for the recordthread */
- char mqname[16]; /* Name of our message queue */
- pthread_t record_id; /* Thread ID of the recordthread */
- int crefs; /* Number of references to the recorder */
- sem_t sem; /* Thread sync semaphore */
- int fd; /* File descriptor of open file */
- char device[CONFIG_NAME_MAX]; /* Preferred audio device */
+ int state; /* Current recorder state */
+ int dev_fd; /* File descriptor of active device */
+ mqd_t mq; /* Message queue for the recordthread */
+ char mqname[16]; /* Name of our message queue */
+ pthread_t record_id; /* Thread ID of the recordthread */
+ int crefs; /* Number of references to the recorder */
+ pthread_mutex_t mutex; /* Thread sync mutex */
+ int fd; /* File descriptor of open file */
+ char device[CONFIG_NAME_MAX]; /* Preferred audio device */
#ifdef CONFIG_AUDIO_MULTI_SESSION
- FAR void *session; /* Session assignment from device */
+ FAR void *session; /* Session assignment from device */
#endif
};
diff --git a/system/nxlooper/nxlooper.c b/system/nxlooper/nxlooper.c
index 5efb39d7f..90cec746e 100644
--- a/system/nxlooper/nxlooper.c
+++ b/system/nxlooper/nxlooper.c
@@ -670,9 +670,7 @@ err_out:
/* Cleanup */
- while (sem_wait(&plooper->sem) < 0)
- {
- }
+ pthread_mutex_lock(&plooper->mutex);
close(plooper->playdev_fd); /* Close the play device */
close(plooper->recorddev_fd); /* Close the record device */
@@ -682,7 +680,7 @@ err_out:
mq_unlink(plooper->mqname); /* Unlink the message queue */
plooper->loopstate = NXLOOPER_STATE_IDLE;
- sem_post(&plooper->sem); /* Release the semaphore */
+ pthread_mutex_unlock(&plooper->mutex);
audinfo("Exit\n");
@@ -706,10 +704,7 @@ int nxlooper_setvolume(FAR struct nxlooper_s *plooper, uint16_t volume)
struct audio_caps_desc_s cap_desc;
int ret;
- /* Thread sync using the semaphore */
-
- while (sem_wait(&plooper->sem) < 0)
- ;
+ pthread_mutex_lock(&plooper->mutex);
/* If we are currently looping, then we need to post a message to
* the loopthread to perform the volume change operation. If we
@@ -735,7 +730,7 @@ int nxlooper_setvolume(FAR struct nxlooper_s *plooper, uint16_t volume)
DEBUGASSERT(errcode > 0);
auderr("ERROR: AUDIOIOC_CONFIGURE ioctl failed: %d\n", errcode);
- sem_post(&plooper->sem);
+ pthread_mutex_unlock(&plooper->mutex);
return -errcode;
}
}
@@ -743,7 +738,7 @@ int nxlooper_setvolume(FAR struct nxlooper_s *plooper, uint16_t volume)
/* Store the volume setting */
plooper->volume = volume;
- sem_post(&plooper->sem);
+ pthread_mutex_unlock(&plooper->mutex);
return OK;
}
@@ -915,14 +910,14 @@ int nxlooper_stop(FAR struct nxlooper_s *plooper)
/* Validate we are not in IDLE state */
- sem_wait(&plooper->sem); /* Get the semaphore */
+ pthread_mutex_lock(&plooper->mutex);
if (plooper->loopstate == NXLOOPER_STATE_IDLE)
{
- sem_post(&plooper->sem); /* Release the semaphore */
+ pthread_mutex_unlock(&plooper->mutex);
return OK;
}
- sem_post(&plooper->sem);
+ pthread_mutex_unlock(&plooper->mutex);
/* Notify the loopback thread that it needs to cancel the loopback */
@@ -1207,7 +1202,7 @@ FAR struct nxlooper_s *nxlooper_create(void)
plooper->precordses = NULL;
#endif
- sem_init(&plooper->sem, 0, 1);
+ pthread_mutex_init(&plooper->mutex, NULL);
return plooper;
}
@@ -1230,45 +1225,23 @@ void nxlooper_release(FAR struct nxlooper_s *plooper)
int refcount;
FAR void *value;
- /* Grab the semaphore */
-
- while (sem_wait(&plooper->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&plooper->mutex);
/* Check if there was a previous thread and join it if there was */
if (plooper->loop_id != 0)
{
- sem_post(&plooper->sem);
+ pthread_mutex_unlock(&plooper->mutex);
pthread_join(plooper->loop_id, &value);
plooper->loop_id = 0;
- while (sem_wait(&plooper->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&plooper->mutex);
}
/* Reduce the reference count */
refcount = plooper->crefs--;
- sem_post(&plooper->sem);
+ pthread_mutex_unlock(&plooper->mutex);
/* If the ref count *was* one, then free the context */
@@ -1292,24 +1265,12 @@ void nxlooper_release(FAR struct nxlooper_s *plooper)
void nxlooper_reference(FAR struct nxlooper_s *plooper)
{
- /* Grab the semaphore */
-
- while (sem_wait(&plooper->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&plooper->mutex);
/* Increment the reference count */
plooper->crefs++;
- sem_post(&plooper->sem);
+ pthread_mutex_unlock(&plooper->mutex);
}
/****************************************************************************
diff --git a/system/nxplayer/nxplayer.c b/system/nxplayer/nxplayer.c
index ea80e1e44..bde36980d 100644
--- a/system/nxplayer/nxplayer.c
+++ b/system/nxplayer/nxplayer.c
@@ -1113,8 +1113,7 @@ err_out:
/* Cleanup */
- while (sem_wait(&pplayer->sem) < 0)
- ;
+ pthread_mutex_lock(&pplayer->mutex);
/* Close the files */
@@ -1131,7 +1130,7 @@ err_out:
pplayer->ops = NULL; /* Clear offload parser */
pplayer->state = NXPLAYER_STATE_IDLE; /* Go to IDLE */
- sem_post(&pplayer->sem); /* Release the semaphore */
+ pthread_mutex_unlock(&pplayer->mutex);
/* The playthread is done with the context. Release it, which may
* actually cause the context to be freed if the creator has already
@@ -1162,10 +1161,7 @@ int nxplayer_setvolume(FAR struct nxplayer_s *pplayer, uint16_t volume)
struct audio_caps_desc_s cap_desc;
int ret;
- /* Thread sync using the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- ;
+ pthread_mutex_lock(&pplayer->mutex);
/* If we are currently playing, then we need to post a message to
* the playthread to perform the volume change operation. If we
@@ -1192,7 +1188,7 @@ int nxplayer_setvolume(FAR struct nxplayer_s *pplayer, uint16_t volume)
DEBUGASSERT(errcode > 0);
auderr("ERROR: AUDIOIOC_CONFIGURE ioctl failed: %d\n", errcode);
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
return -errcode;
}
}
@@ -1200,7 +1196,7 @@ int nxplayer_setvolume(FAR struct nxplayer_s *pplayer, uint16_t volume)
/* Store the volume setting */
pplayer->volume = volume;
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
return OK;
}
@@ -1250,10 +1246,7 @@ int nxplayer_setbass(FAR struct nxplayer_s *pplayer, uint8_t level)
{
struct audio_caps_desc_s cap_desc;
- /* Thread sync using the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- ;
+ pthread_mutex_lock(&pplayer->mutex);
/* If we are currently playing, then we need to post a message to
* the playthread to perform the volume change operation. If we
@@ -1279,7 +1272,7 @@ int nxplayer_setbass(FAR struct nxplayer_s *pplayer, uint8_t level)
pplayer->bass = level;
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
return -ENOENT;
}
@@ -1302,10 +1295,7 @@ int nxplayer_settreble(FAR struct nxplayer_s *pplayer, uint8_t level)
{
struct audio_caps_desc_s cap_desc;
- /* Thread sync using the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- ;
+ pthread_mutex_lock(&pplayer->mutex);
/* If we are currently playing, then we need to post a message to
* the playthread to perform the volume change operation. If we
@@ -1331,7 +1321,7 @@ int nxplayer_settreble(FAR struct nxplayer_s *pplayer, uint8_t level)
pplayer->treble = level;
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
return -ENOENT;
}
@@ -1350,10 +1340,7 @@ int nxplayer_setbalance(FAR struct nxplayer_s *pplayer, uint16_t balance)
{
struct audio_caps_desc_s cap_desc;
- /* Thread sync using the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- ;
+ pthread_mutex_lock(&pplayer->mutex);
/* If we are currently playing, then we need to post a message to
* the playthread to perform the volume change operation. If we
@@ -1379,7 +1366,7 @@ int nxplayer_setbalance(FAR struct nxplayer_s *pplayer, uint16_t balance)
pplayer->balance = balance;
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
return -ENOENT;
}
@@ -1689,14 +1676,14 @@ int nxplayer_stop(FAR struct nxplayer_s *pplayer)
/* Validate we are not in IDLE state */
- sem_wait(&pplayer->sem); /* Get the semaphore */
+ pthread_mutex_lock(&pplayer->mutex);
if (pplayer->state == NXPLAYER_STATE_IDLE)
{
- sem_post(&pplayer->sem); /* Release the semaphore */
+ pthread_mutex_unlock(&pplayer->mutex);
return OK;
}
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
/* Notify the playback thread that it needs to cancel the playback */
@@ -2150,7 +2137,8 @@ FAR struct nxplayer_s *nxplayer_create(void)
strncpy(pplayer->mediadir, CONFIG_NXPLAYER_DEFAULT_MEDIADIR,
sizeof(pplayer->mediadir));
#endif
- sem_init(&pplayer->sem, 0, 1);
+
+ pthread_mutex_init(&pplayer->mutex, NULL);
return pplayer;
}
@@ -2173,45 +2161,23 @@ void nxplayer_release(FAR struct nxplayer_s *pplayer)
int refcount;
FAR void *value;
- /* Grab the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&pplayer->mutex);
/* Check if there was a previous thread and join it if there was */
if (pplayer->play_id != 0)
{
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
pthread_join(pplayer->play_id, &value);
pplayer->play_id = 0;
- while (sem_wait(&pplayer->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != -EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&pplayer->mutex);
}
/* Reduce the reference count */
refcount = pplayer->crefs--;
- sem_post(&pplayer->sem);
+ pthread_mutex_unlock(&pplayer->mutex);
/* If the ref count *was* one, then free the context */
@@ -2235,67 +2201,12 @@ void nxplayer_release(FAR struct nxplayer_s *pplayer)
void nxplayer_reference(FAR struct nxplayer_s *pplayer)
{
- /* Grab the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != -EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&pplayer->mutex);
/* Increment the reference count */
pplayer->crefs++;
- sem_post(&pplayer->sem);
-}
-
-/****************************************************************************
- * Name: nxplayer_detach
- *
- * nxplayer_detach() detaches from the playthread to make it independent
- * so the caller can abandon the context while the file is still
- * being played.
- *
- * Input Parameters:
- * pplayer Pointer to the NxPlayer context
- *
- * Returned values: None
- *
- ****************************************************************************/
-
-void nxplayer_detach(FAR struct nxplayer_s *pplayer)
-{
-#if 0
- /* Grab the semaphore */
-
- while (sem_wait(&pplayer->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != -EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
-
- if (pplayer->play_id != NULL)
- {
- /* Do a pthread detach */
-
- pthread_detach(pplayer->play_id);
- pplayer->play_id = NULL;
- }
-
- sem_post(&pplayer->sem);
-#endif
+ pthread_mutex_unlock(&pplayer->mutex);
}
/****************************************************************************
diff --git a/system/nxplayer/nxplayer_main.c b/system/nxplayer/nxplayer_main.c
index f28cfe239..052fadab8 100644
--- a/system/nxplayer/nxplayer_main.c
+++ b/system/nxplayer/nxplayer_main.c
@@ -837,8 +837,6 @@ int main(int argc, FAR char *argv[])
/* Release the NxPlayer context */
- /* nxplayer_detach(pplayer); */
-
nxplayer_release(pplayer);
return OK;
diff --git a/system/nxrecorder/nxrecorder.c b/system/nxrecorder/nxrecorder.c
index 7f4054537..83fdd3186 100644
--- a/system/nxrecorder/nxrecorder.c
+++ b/system/nxrecorder/nxrecorder.c
@@ -566,9 +566,7 @@ err_out:
/* Cleanup */
- while (sem_wait(&precorder->sem) < 0)
- {
- }
+ pthread_mutex_lock(&precorder->mutex);
/* Close the files */
@@ -584,7 +582,7 @@ err_out:
mq_unlink(precorder->mqname); /* Unlink the message queue */
precorder->state = NXRECORDER_STATE_IDLE; /* Go to IDLE */
- sem_post(&precorder->sem); /* Release the semaphore */
+ pthread_mutex_unlock(&precorder->mutex);
/* The record thread is done with the context. Release it, which may
* actually cause the context to be freed if the creator has already
@@ -718,14 +716,14 @@ int nxrecorder_stop(FAR struct nxrecorder_s *precorder)
/* Validate we are not in IDLE state */
- sem_wait(&precorder->sem); /* Get the semaphore */
+ pthread_mutex_lock(&precorder->mutex);
if (precorder->state == NXRECORDER_STATE_IDLE)
{
- sem_post(&precorder->sem); /* Release the semaphore */
+ pthread_mutex_unlock(&precorder->mutex);
return OK;
}
- sem_post(&precorder->sem);
+ pthread_mutex_unlock(&precorder->mutex);
/* Notify the recordback thread that it needs to cancel the recordback */
@@ -976,7 +974,7 @@ FAR struct nxrecorder_s *nxrecorder_create(void)
precorder->session = NULL;
#endif
- sem_init(&precorder->sem, 0, 1);
+ pthread_mutex_init(&precorder->mutex, NULL);
return precorder;
}
@@ -999,50 +997,29 @@ void nxrecorder_release(FAR struct nxrecorder_s *precorder)
int refcount;
FAR void *value;
- /* Grab the semaphore */
-
- while (sem_wait(&precorder->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&precorder->mutex);
/* Check if there was a previous thread and join it if there was */
if (precorder->record_id != 0)
{
- sem_post(&precorder->sem);
+ pthread_mutex_unlock(&precorder->mutex);
pthread_join(precorder->record_id, &value);
precorder->record_id = 0;
- while (sem_wait(&precorder->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&precorder->mutex);
}
/* Reduce the reference count */
refcount = precorder->crefs--;
- sem_post(&precorder->sem);
+ pthread_mutex_unlock(&precorder->mutex);
/* If the ref count *was* one, then free the context */
if (refcount == 1)
{
+ pthread_mutex_destroy(&precorder->mutex);
free(precorder);
}
}
@@ -1061,22 +1038,10 @@ void nxrecorder_release(FAR struct nxrecorder_s *precorder)
void nxrecorder_reference(FAR struct nxrecorder_s *precorder)
{
- /* Grab the semaphore */
-
- while (sem_wait(&precorder->sem) < 0)
- {
- int errcode = errno;
- DEBUGASSERT(errcode > 0);
-
- if (errcode != EINTR)
- {
- auderr("ERROR: sem_wait failed: %d\n", errcode);
- return;
- }
- }
+ pthread_mutex_lock(&precorder->mutex);
/* Increment the reference count */
precorder->crefs++;
- sem_post(&precorder->sem);
+ pthread_mutex_unlock(&precorder->mutex);
}
[nuttx-apps] 02/02: system/media: fix race condion on pthread id
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git
commit 0bf87e7893ea409edc24f7f78532e1632080262b
Author: ligd <li...@xiaomi.com>
AuthorDate: Tue Jan 3 20:35:46 2023 +0800
system/media: fix race condion on pthread id
Signed-off-by: ligd <li...@xiaomi.com>
---
system/nxcamera/nxcamera.c | 56 +++++++++++++++++++++++++++---------------
system/nxlooper/nxlooper.c | 52 +++++++++++++++++++++++++--------------
system/nxplayer/nxplayer.c | 52 +++++++++++++++++++++++++--------------
system/nxrecorder/nxrecorder.c | 52 +++++++++++++++++++++++++--------------
4 files changed, 138 insertions(+), 74 deletions(-)
diff --git a/system/nxcamera/nxcamera.c b/system/nxcamera/nxcamera.c
index c1d90ac48..5cb9c1341 100644
--- a/system/nxcamera/nxcamera.c
+++ b/system/nxcamera/nxcamera.c
@@ -167,6 +167,36 @@ static int nxcamera_opendevice(FAR struct nxcamera_s *pcam)
return -ENODEV;
}
+/****************************************************************************
+ * Name: nxcameraer_jointhread
+ ****************************************************************************/
+
+static void nxcamera_jointhread(FAR struct nxcamera_s *pcam)
+{
+ FAR void *value;
+ int id = 0;
+
+ if (gettid() == pcam->loop_id)
+ {
+ return;
+ }
+
+ pthread_mutex_lock(&pcam->mutex);
+
+ if (pcam->loop_id > 0)
+ {
+ id = pcam->loop_id;
+ pcam->loop_id = 0;
+ }
+
+ pthread_mutex_unlock(&pcam->mutex);
+
+ if (id > 0)
+ {
+ pthread_join(id, &value);
+ }
+}
+
/****************************************************************************
* Name: nxcamera_loopthread
*
@@ -477,7 +507,6 @@ int nxcamera_setfile(FAR struct nxcamera_s *pcam, FAR const char *pfile,
int nxcamera_stop(FAR struct nxcamera_s *pcam)
{
struct video_msg_s term_msg;
- FAR void *value;
DEBUGASSERT(pcam != NULL);
@@ -501,8 +530,7 @@ int nxcamera_stop(FAR struct nxcamera_s *pcam)
/* Join the thread. The thread will do all the cleanup. */
- pthread_join(pcam->loop_id, &value);
- pcam->loop_id = 0;
+ nxcamera_jointhread(pcam);
return OK;
}
@@ -536,7 +564,6 @@ int nxcamera_stream(FAR struct nxcamera_s *pcam,
struct mq_attr attr;
struct sched_param sparam;
pthread_attr_t tattr;
- FAR void *value;
int ret;
int i;
struct v4l2_buffer buf;
@@ -681,10 +708,7 @@ int nxcamera_stream(FAR struct nxcamera_s *pcam,
* to perform clean-up.
*/
- if (pcam->loop_id != 0)
- {
- pthread_join(pcam->loop_id, &value);
- }
+ nxcamera_jointhread(pcam);
pthread_attr_init(&tattr);
sparam.sched_priority = sched_get_priority_max(SCHED_FIFO) - 9;
@@ -792,23 +816,15 @@ FAR struct nxcamera_s *nxcamera_create(void)
void nxcamera_release(FAR struct nxcamera_s *pcam)
{
- FAR void *value;
int refcount;
- /* Lock the mutex */
-
- pthread_mutex_lock(&pcam->mutex);
-
/* Check if there was a previous thread and join it if there was */
- if (pcam->loop_id != 0)
- {
- pthread_mutex_unlock(&pcam->mutex);
- pthread_join(pcam->loop_id, &value);
- pcam->loop_id = 0;
+ nxcamera_jointhread(pcam);
- pthread_mutex_lock(&pcam->mutex);
- }
+ /* Lock the mutex */
+
+ pthread_mutex_lock(&pcam->mutex);
/* Reduce the reference count */
diff --git a/system/nxlooper/nxlooper.c b/system/nxlooper/nxlooper.c
index 90cec746e..3e03afea4 100644
--- a/system/nxlooper/nxlooper.c
+++ b/system/nxlooper/nxlooper.c
@@ -308,6 +308,36 @@ static int nxlooper_enqueueplaybuffer(FAR struct nxlooper_s *plooper,
return OK;
}
+/****************************************************************************
+ * Name: nxlooper_jointhread
+ ****************************************************************************/
+
+static void nxlooper_jointhread(FAR struct nxlooper_s *plooper)
+{
+ FAR void *value;
+ int id = 0;
+
+ if (gettid() == plooper->loop_id)
+ {
+ return;
+ }
+
+ pthread_mutex_lock(&plooper->mutex);
+
+ if (plooper->loop_id > 0)
+ {
+ id = plooper->loop_id;
+ plooper->loop_id = 0;
+ }
+
+ pthread_mutex_unlock(&plooper->mutex);
+
+ if (id > 0)
+ {
+ pthread_join(id, &value);
+ }
+}
+
/****************************************************************************
* Name: nxlooper_thread_loopthread
*
@@ -904,7 +934,6 @@ int nxlooper_setdevice(FAR struct nxlooper_s *plooper,
int nxlooper_stop(FAR struct nxlooper_s *plooper)
{
struct audio_msg_s term_msg;
- FAR void *value;
DEBUGASSERT(plooper != NULL);
@@ -928,8 +957,7 @@ int nxlooper_stop(FAR struct nxlooper_s *plooper)
/* Join the thread. The thread will do all the cleanup. */
- pthread_join(plooper->loop_id, &value);
- plooper->loop_id = 0;
+ nxlooper_jointhread(plooper);
return OK;
}
@@ -966,7 +994,6 @@ int nxlooper_loopraw(FAR struct nxlooper_s *plooper,
pthread_attr_t tattr;
struct audio_caps_desc_s cap_desc;
struct ap_buffer_info_s buf_info;
- FAR void *value;
int ret;
DEBUGASSERT(plooper != NULL);
@@ -1098,10 +1125,7 @@ int nxlooper_loopraw(FAR struct nxlooper_s *plooper,
* to perform clean-up.
*/
- if (plooper->loop_id != 0)
- {
- pthread_join(plooper->loop_id, &value);
- }
+ nxlooper_jointhread(plooper);
pthread_attr_init(&tattr);
sparam.sched_priority = sched_get_priority_max(SCHED_FIFO) - 9;
@@ -1223,20 +1247,12 @@ FAR struct nxlooper_s *nxlooper_create(void)
void nxlooper_release(FAR struct nxlooper_s *plooper)
{
int refcount;
- FAR void *value;
-
- pthread_mutex_lock(&plooper->mutex);
/* Check if there was a previous thread and join it if there was */
- if (plooper->loop_id != 0)
- {
- pthread_mutex_unlock(&plooper->mutex);
- pthread_join(plooper->loop_id, &value);
- plooper->loop_id = 0;
+ nxlooper_jointhread(plooper);
- pthread_mutex_lock(&plooper->mutex);
- }
+ pthread_mutex_lock(&plooper->mutex);
/* Reduce the reference count */
diff --git a/system/nxplayer/nxplayer.c b/system/nxplayer/nxplayer.c
index bde36980d..c8f534e4e 100644
--- a/system/nxplayer/nxplayer.c
+++ b/system/nxplayer/nxplayer.c
@@ -718,6 +718,36 @@ static int nxplayer_enqueuebuffer(FAR struct nxplayer_s *pplayer,
return OK;
}
+/****************************************************************************
+ * Name: nxplayer_jointhread
+ ****************************************************************************/
+
+static void nxplayer_jointhread(FAR struct nxplayer_s *pplayer)
+{
+ FAR void *value;
+ int id = 0;
+
+ if (gettid() == pplayer->play_id)
+ {
+ return;
+ }
+
+ pthread_mutex_lock(&pplayer->mutex);
+
+ if (pplayer->play_id > 0)
+ {
+ id = pplayer->play_id;
+ pplayer->play_id = 0;
+ }
+
+ pthread_mutex_unlock(&pplayer->mutex);
+
+ if (id > 0)
+ {
+ pthread_join(id, &value);
+ }
+}
+
/****************************************************************************
* Name: nxplayer_thread_playthread
*
@@ -1670,7 +1700,6 @@ int nxplayer_setdevice(FAR struct nxplayer_s *pplayer,
int nxplayer_stop(FAR struct nxplayer_s *pplayer)
{
struct audio_msg_s term_msg;
- FAR void *value;
DEBUGASSERT(pplayer != NULL);
@@ -1694,8 +1723,7 @@ int nxplayer_stop(FAR struct nxplayer_s *pplayer)
/* Join the thread. The thread will do all the cleanup. */
- pthread_join(pplayer->play_id, &value);
- pplayer->play_id = 0;
+ nxplayer_jointhread(pplayer);
return OK;
}
@@ -1738,7 +1766,6 @@ static int nxplayer_playinternal(FAR struct nxplayer_s *pplayer,
struct mq_attr attr;
struct sched_param sparam;
pthread_attr_t tattr;
- FAR void *value;
struct audio_caps_desc_s cap_desc;
struct ap_buffer_info_s buf_info;
#ifdef CONFIG_NXPLAYER_INCLUDE_MEDIADIR
@@ -1936,10 +1963,7 @@ static int nxplayer_playinternal(FAR struct nxplayer_s *pplayer,
* to perform clean-up.
*/
- if (pplayer->play_id != 0)
- {
- pthread_join(pplayer->play_id, &value);
- }
+ nxplayer_jointhread(pplayer);
/* Start the playfile thread to stream the media file to the
* audio device.
@@ -2159,20 +2183,12 @@ FAR struct nxplayer_s *nxplayer_create(void)
void nxplayer_release(FAR struct nxplayer_s *pplayer)
{
int refcount;
- FAR void *value;
-
- pthread_mutex_lock(&pplayer->mutex);
/* Check if there was a previous thread and join it if there was */
- if (pplayer->play_id != 0)
- {
- pthread_mutex_unlock(&pplayer->mutex);
- pthread_join(pplayer->play_id, &value);
- pplayer->play_id = 0;
+ nxplayer_jointhread(pplayer);
- pthread_mutex_lock(&pplayer->mutex);
- }
+ pthread_mutex_lock(&pplayer->mutex);
/* Reduce the reference count */
diff --git a/system/nxrecorder/nxrecorder.c b/system/nxrecorder/nxrecorder.c
index 83fdd3186..9b6cb4372 100644
--- a/system/nxrecorder/nxrecorder.c
+++ b/system/nxrecorder/nxrecorder.c
@@ -215,6 +215,36 @@ static int nxrecorder_enqueuebuffer(FAR struct nxrecorder_s *precorder,
return OK;
}
+/****************************************************************************
+ * Name: nxrecorder_jointhread
+ ****************************************************************************/
+
+static void nxrecorder_jointhread(FAR struct nxrecorder_s *precorder)
+{
+ FAR void *value;
+ int id = 0;
+
+ if (gettid() == precorder->record_id)
+ {
+ return;
+ }
+
+ pthread_mutex_lock(&precorder->mutex);
+
+ if (precorder->record_id > 0)
+ {
+ id = precorder->record_id;
+ precorder->record_id = 0;
+ }
+
+ pthread_mutex_unlock(&precorder->mutex);
+
+ if (id > 0)
+ {
+ pthread_join(id, &value);
+ }
+}
+
/****************************************************************************
* Name: nxrecorder_thread_recordthread
*
@@ -710,7 +740,6 @@ int nxrecorder_setdevice(FAR struct nxrecorder_s *precorder,
int nxrecorder_stop(FAR struct nxrecorder_s *precorder)
{
struct audio_msg_s term_msg;
- FAR void *value;
DEBUGASSERT(precorder != NULL);
@@ -734,8 +763,7 @@ int nxrecorder_stop(FAR struct nxrecorder_s *precorder)
/* Join the thread. The thread will do all the cleanup. */
- pthread_join(precorder->record_id, &value);
- precorder->record_id = 0;
+ nxrecorder_jointhread(precorder);
return OK;
}
@@ -773,7 +801,6 @@ int nxrecorder_recordraw(FAR struct nxrecorder_s *precorder,
pthread_attr_t tattr;
struct audio_caps_desc_s cap_desc;
struct ap_buffer_info_s buf_info;
- FAR void *value;
int ret;
DEBUGASSERT(precorder != NULL);
@@ -884,10 +911,7 @@ int nxrecorder_recordraw(FAR struct nxrecorder_s *precorder,
* to perform clean-up.
*/
- if (precorder->record_id != 0)
- {
- pthread_join(precorder->record_id, &value);
- }
+ nxrecorder_jointhread(precorder);
/* Start the recordfile thread to stream the media file to the
* audio device.
@@ -995,20 +1019,12 @@ FAR struct nxrecorder_s *nxrecorder_create(void)
void nxrecorder_release(FAR struct nxrecorder_s *precorder)
{
int refcount;
- FAR void *value;
-
- pthread_mutex_lock(&precorder->mutex);
/* Check if there was a previous thread and join it if there was */
- if (precorder->record_id != 0)
- {
- pthread_mutex_unlock(&precorder->mutex);
- pthread_join(precorder->record_id, &value);
- precorder->record_id = 0;
+ nxrecorder_jointhread(precorder);
- pthread_mutex_lock(&precorder->mutex);
- }
+ pthread_mutex_lock(&precorder->mutex);
/* Reduce the reference count */