On Sun, 24 Nov 2013 19:15:09 -0500, Tristan Matthews
<[email protected]> wrote:
> Fixes https://bugzilla.libav.org/show_bug.cgi?id=312
> ---
> libavdevice/jack_audio.c | 55
> +++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 40 insertions(+), 15 deletions(-)
>
> diff --git a/libavdevice/jack_audio.c b/libavdevice/jack_audio.c
> index c261514..ef6e0fb 100644
> --- a/libavdevice/jack_audio.c
> +++ b/libavdevice/jack_audio.c
> @@ -21,7 +21,7 @@
> */
>
> #include "config.h"
> -#include <semaphore.h>
> +#include <pthread.h>
> #include <jack/jack.h>
>
> #include "libavutil/log.h"
> @@ -42,7 +42,8 @@ typedef struct {
> AVClass *class;
> jack_client_t * client;
> int activated;
> - sem_t packet_count;
> + pthread_mutex_t pkt_mutex;
> + pthread_cond_t pkt_cond;
> jack_nframes_t sample_rate;
> jack_nframes_t buffer_size;
> jack_port_t ** ports;
> @@ -107,9 +108,9 @@ static int process_callback(jack_nframes_t nframes,
> void *arg)
> /* Timestamp the packet with the cycle start time minus the average
> latency */
> pkt.pts = (cycle_time - (double) latency / (self->nports *
> self->sample_rate)) * 1000000.0;
>
> - /* Send the now filled packet back, and increase packet counter */
> + /* Send the now filled packet back, and signal that it's ready */
> av_fifo_generic_write(self->filled_pkts, &pkt, sizeof(pkt), NULL);
> - sem_post(&self->packet_count);
> + pthread_cond_signal(&self->pkt_cond);
Signalling a condition variable without any mutex operation in the
vicinity raises both of my eyebrows.
>
> return 0;
> }
> @@ -160,7 +161,8 @@ static int start_jack(AVFormatContext *context)
> return AVERROR(EIO);
> }
>
> - sem_init(&self->packet_count, 0, 0);
> + pthread_mutex_init(&self->pkt_mutex, NULL);
> + pthread_cond_init(&self->pkt_cond, NULL);
>
> self->sample_rate = jack_get_sample_rate(self->client);
> self->ports = av_malloc(self->nports * sizeof(*self->ports));
> @@ -225,7 +227,8 @@ static void stop_jack(JackData *self)
> jack_deactivate(self->client);
> jack_client_close(self->client);
> }
> - sem_destroy(&self->packet_count);
> + pthread_cond_destroy(&self->pkt_cond);
> + pthread_mutex_destroy(&self->pkt_mutex);
> free_pkt_fifo(self->new_pkts);
> free_pkt_fifo(self->filled_pkts);
> av_freep(&self->ports);
> @@ -284,18 +287,40 @@ static int audio_read_packet(AVFormatContext
> *context, AVPacket *pkt)
>
> /* Wait for a packet coming back from process_callback(), if one
> isn't available yet */
> timeout.tv_sec = av_gettime() / 1000000 + 2;
> - if (sem_timedwait(&self->packet_count, &timeout)) {
> - if (errno == ETIMEDOUT) {
> - av_log(context, AV_LOG_ERROR,
> - "Input error: timed out when waiting for JACK
process
> callback output\n");
> - } else {
> - av_log(context, AV_LOG_ERROR, "Error while waiting for
audio
> packet: %s\n",
> +
> + /* Only wait if there's nothing to read yet */
> + if (!av_fifo_size(self->filled_pkts)) {
> +
> + if (pthread_mutex_lock(&self->pkt_mutex)) {
Cleanly handling pthread_mutex_lock error is _useless_. If that function
does fail, the code is _already_ in undefined land(*). So, IMHO, aborting
the process is the only reasonable thing to do.
(*) There is one exception: if the mutex is error-checking *and* already
locked by the calling thread, then EDEADLK will be returned.
> + av_log(context, AV_LOG_ERROR, "Error while trying to lock
> mutex: %s\n",
> strerror(errno));
> + return AVERROR(EIO);
> }
> - if (!self->client)
> - av_log(context, AV_LOG_ERROR, "Input error: JACK server is
> gone\n");
>
> - return AVERROR(EIO);
> + if (pthread_cond_timedwait(&self->pkt_cond, &self->pkt_mutex,
> &timeout)) {
> + if (errno == ETIMEDOUT) {
> + av_log(context, AV_LOG_ERROR,
> + "Input error: timed out when waiting for JACK
> process callback output\n");
> + } else {
> + av_log(context, AV_LOG_ERROR, "Error while waiting for
> audio packet: %s\n",
> + strerror(errno));
Ditto. The only well-defined error is ETIMEDOUT.
> + }
> + if (!self->client)
> + av_log(context, AV_LOG_ERROR, "Input error: JACK server
> is gone\n");
> +
> + if (pthread_mutex_unlock(&self->pkt_mutex)) {
And ditto.
> + av_log(context, AV_LOG_ERROR, "Error while trying to
> unlock mutex: %s\n",
> + strerror(errno));
> + }
> +
> + return AVERROR(EIO);
> + }
> +
> + if (pthread_mutex_unlock(&self->pkt_mutex)) {
> + av_log(context, AV_LOG_ERROR, "Error while trying to unlock
> mutex: %s\n",
> + strerror(errno));
> + return AVERROR(EIO);
> + }
> }
>
> if (self->pkt_xrun) {
--
Rémi Denis-Courmont
Sent from my collocated server
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
