On Tue, Dec 01, 2015 at 05:11:06PM +0100, Nicolas George wrote:
> Le decadi 10 frimaire, an CCXXIV, Clement Boesch a écrit :
> > From: Clément Bœsch <clem...@stupeflix.com>
> >
> > ---
> > libavutil/threadmessage.c | 37 ++++++++++++++++++++++++++++++++++---
> > libavutil/threadmessage.h | 21 ++++++++++++++++++---
> > 2 files changed, 52 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavutil/threadmessage.c b/libavutil/threadmessage.c
> > index b7fcbe2..87ce8dc 100644
> > --- a/libavutil/threadmessage.c
> > +++ b/libavutil/threadmessage.c
> > @@ -40,14 +40,16 @@ struct AVThreadMessageQueue {
> > int err_send;
> > int err_recv;
> > unsigned elsize;
> > + void (*free_func)(void *msg);
> > #else
> > int dummy;
> > #endif
> > };
> >
> > -int av_thread_message_queue_alloc(AVThreadMessageQueue **mq,
> > - unsigned nelem,
> > - unsigned elsize)
>
> > +int av_thread_message_queue_alloc2(AVThreadMessageQueue **mq,
> > + unsigned nelem,
> > + unsigned elsize,
> > + void (*free_func)(void *msg))
>
> I must say I am not very fond of this aspect of the change, because of
> foobar2 and numerous function arguments.
>
> What about having the application set queue->free_func with a second
> function if needed, either directly or through
> av_thread_message_queue_set_free_func()?
>
Yeah, sure, no opinion really. Will send a new patch.
[...]
> > +/**
> > + * Flush the message queue
> > + */
> > +void av_thread_message_flush(AVThreadMessageQueue *mq);
>
> Can you explain in the doxy (and to me) how it is better than:
>
> while (av_thread_message_queue_recv_locked(mq, msg,
> AV_THREAD_MESSAGE_NONBLOCK))
> free_func(msg);
>
current:
int used, off;
pthread_mutex_lock(&mq->lock);
used = av_fifo_size(mq->fifo);
if (mq->free_func) {
for (off = 0; off < used; off += mq->elsize) {
void *msg;
av_fifo_generic_peek_at(mq->fifo, &msg, off, mq->elsize, NULL);
mq->free_func(msg);
}
}
av_fifo_drain(mq->fifo, used);
pthread_cond_broadcast(&mq->cond);
pthread_mutex_unlock(&mq->lock);
yours:
void *msg;
pthread_mutex_lock(&mq->lock);
while (av_thread_message_queue_recv_locked(mq, msg,
AV_THREAD_MESSAGE_NONBLOCK))
if (mq->free_func)
mq->free_func(msg);
pthread_mutex_unlock(&mq->lock);
Your suggestion is twice smaller. I don't mind that much but I guess I like
saving a few checks and preventing a bunch of signal broadcasting. Also,
passing the AV_THREAD_MESSAGE_NONBLOCK seems a bit like a (smart) hack to me. I
will pick your solution (which I admit haven't thought of initially) if you
insist, but I prefer my version.
[...]
--
Clément B.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
