[FFmpeg-devel] [PATCH] Message queue API

[Clément Bœsch]

On Sun, May 04, 2014 at 04:59:22PM +0200, Nicolas George wrote:
[...]
> +struct AVThreadMessageQueue {
> +#if HAVE_THREADS
> +    AVFifoBuffer *fifo;
> +    pthread_mutex_t lock;
> +    pthread_cond_t cond;
> +    int err_send;
> +    int err_recv;
> +    unsigned elsize;
> +#endif

I'm not sure an empty struct is valid in C, you may need to add a dummy
field or something to avoid chocking with random compilers.

> +};
> +
> +int av_thread_message_queue_alloc(AVThreadMessageQueue **mq,

> +                                  unsigned elsize,
> +                                  unsigned nelem)

Those should probably be size_t, but it's not important.

BTW, I know this is a pain to change now, but the number of element is
often (always?) put before the element size.

> +{
> +#if HAVE_THREADS
> +    AVThreadMessageQueue *rmq;
> +    int ret = 0;
> +
> +    if (nelem > INT_MAX / elsize)
> +        return AVERROR(EINVAL);
> +    if (!(rmq = av_mallocz(sizeof(*rmq))))
> +        return AVERROR(ENOMEM);
> +    if ((ret = pthread_mutex_init(&rmq->lock, NULL))) {
> +        av_free(rmq);
> +        return AVERROR(ret);
> +    }
> +    if ((ret = pthread_cond_init(&rmq->cond, NULL))) {
> +        pthread_mutex_destroy(&rmq->lock);
> +        av_free(rmq);
> +        return AVERROR(ret);
> +    }

> +    if (!(rmq->fifo = av_fifo_alloc(elsize * nelem))) {

Note for later: we should add a av_fifo_alloc2(size_t nmemb, size_t size)

> +        pthread_cond_destroy(&rmq->cond);
> +        pthread_mutex_destroy(&rmq->lock);
> +        av_free(rmq);
> +        return AVERROR(ret);
> +    }
> +    rmq->elsize = elsize;
> +    *mq = rmq;
> +    return 0;
> +#else
> +    *mq = NULL;
> +    return AVERROR(ENOSYS);
> +#endif /* HAVE_THREADS */
> +}
> +
> +void av_thread_message_queue_free(AVThreadMessageQueue **mq)
> +{
> +#if HAVE_THREADS
> +    if (*mq) {
> +        av_fifo_free((*mq)->fifo);
> +        pthread_cond_destroy(&(*mq)->cond);
> +        pthread_mutex_destroy(&(*mq)->lock);
> +        av_freep(mq);
> +    }
> +#endif
> +}
> +
> +#if HAVE_THREADS
> +

> +static int av_thread_message_queue_send_locked(AVThreadMessageQueue *mq,
> +                                               void *msg,
> +                                               unsigned flags)

It's static so please drop the av_ prefix

> +{
> +    while (!mq->err_send && av_fifo_space(mq->fifo) < mq->elsize) {
> +        if ((flags & AV_THREAD_MESSAGE_NONBLOCK))
> +            return AVERROR(EAGAIN);
> +        pthread_cond_wait(&mq->cond, &mq->lock);
> +    }
> +    if (mq->err_send)
> +        return mq->err_send;
> +    av_fifo_generic_write(mq->fifo, msg, mq->elsize, NULL);
> +    pthread_cond_signal(&mq->cond);
> +    return 0;
> +}
> +

> +static int av_thread_message_queue_recv_locked(AVThreadMessageQueue *mq,
> +                                               void *msg,
> +                                               unsigned flags)

ditto

[...]

No more comment from me.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140504/c96b6356/attachment.asc>