[FFmpeg-devel] [PATCH 1/2] lavu: add thread message API.

[Clément Bœsch]

On Thu, Feb 20, 2014 at 04:22:13PM +0100, Nicolas George wrote:
> TODO minor bump and APIchanges entry.
> 
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
>  libavutil/Makefile        |   2 +
>  libavutil/threadmessage.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/threadmessage.h |  92 +++++++++++++++++++++++++
>  3 files changed, 266 insertions(+)
>  create mode 100644 libavutil/threadmessage.c
>  create mode 100644 libavutil/threadmessage.h
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index e4ee47c..4b173d8 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -50,6 +50,7 @@ HEADERS = adler32.h                                                     \
>            sha.h                                                         \
>            sha512.h                                                      \
>            stereo3d.h                                                    \
> +          threadmessage.h                                               \
>            time.h                                                        \
>            timecode.h                                                    \
>            timestamp.h                                                   \
> @@ -115,6 +116,7 @@ OBJS = adler32.o                                                        \
>         sha.o                                                            \
>         sha512.o                                                         \
>         stereo3d.o                                                       \
> +       threadmessage.o                                                  \
>         time.o                                                           \
>         timecode.o                                                       \
>         tree.o                                                           \
> diff --git a/libavutil/threadmessage.c b/libavutil/threadmessage.c
> new file mode 100644
> index 0000000..5389246
> --- /dev/null
> +++ b/libavutil/threadmessage.c
> @@ -0,0 +1,172 @@
> +/*
> + * Copyright (c) 2014 Nicolas George
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <pthread.h>
> +#include "fifo.h"
> +#include "threadmessage.h"
> +

> +#include <pthread.h>
> +#include "fifo.h"

double include it seems.

Also, I think pthread.h include should be conditional.

> +struct AVThreadMessageQueue {
> +    AVFifoBuffer *fifo;

> +    pthread_mutex_t lock;
> +    pthread_cond_t cond;

This might not compile without pthread

> +    int err_send;
> +    int err_recv;
> +    unsigned elsize;
> +};
> +
> +int av_thread_message_queue_alloc(AVThreadMessageQueue **mq,
> +                                  unsigned elsize,
> +                                  unsigned nelem)
> +{

> +#if HAVE_THREADS

Code in ffmpeg.c seems to rely on HAVE_PTHREADS; is this fine?

[...]
> +int av_thread_message_queue_send(AVThreadMessageQueue *mq,
> +                                 void *msg,
> +                                 unsigned flags)
> +{
> +#if HAVE_THREADS
> +    int ret;
> +
> +    pthread_mutex_lock(&mq->lock);
> +    ret = av_thread_message_queue_send_locked(mq, msg, flags);
> +    pthread_mutex_unlock(&mq->lock);
> +    return ret;
> +#else
> +    return AVERROR(ENOSYS);
> +#endif /* HAVE_THREADS */

so without thread, this will return ENOSYS; won't that cause problems with
your new loop in ffmpeg.c?

[...]
> diff --git a/libavutil/threadmessage.h b/libavutil/threadmessage.h
> new file mode 100644
> index 0000000..0c77bcf
> --- /dev/null
> +++ b/libavutil/threadmessage.h
> @@ -0,0 +1,92 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVUTIL_THREADMESSAGE_H
> +#define AVUTIL_THREADMESSAGE_H
> +
> +#include <pthread.h>

You should not have this include here since the prototypes do not depend
on pthread specific code; if you did, it would have mean that the code can
not compile without threading.

[...]

-- 
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/20140224/6d93b52f/attachment.asc>