[FFmpeg-devel] v4l2: bug #1570 and possible solution

[Michael Niedermayer]

On Tue, Feb 12, 2013 at 12:40:21PM +0100, Giorgio Vazzana wrote:
> 2013/2/12 Michael Niedermayer <michaelni at gmx.at>:
> > If i understand the problem correctly then i would suggest to switch
> > to copying frames into normally allocated packets whenever there is
> > just 1 buffer left available
> 
> This sounds like a good idea. See attached patch (tested and seems to
> work okay).

>  v4l2.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 48 insertions(+), 19 deletions(-)
> 02634d9155fb732e79bc389383efbb59a9a40f04  0001-lavd-v4l2-copy-frames-into-normally-allocated-packet.patch
> From 3e5c057ec73c67944960edd66abf912354449331 Mon Sep 17 00:00:00 2001
> From: Giorgio Vazzana <mywing81 at gmail.com>
> Date: Tue, 12 Feb 2013 12:24:53 +0100
> Subject: [PATCH] lavd/v4l2: copy frames into normally allocated packets whenever there is just one buffer left available
> 
> This will avoid the possibility that we dequeue more buffers than we
> have obtained from the v4l2 driver.
> 
> Fixes ticket #1570
> ---
>  libavdevice/v4l2.c |   67 +++++++++++++++++++++++++++++++++++++--------------
>  1 files changed, 48 insertions(+), 19 deletions(-)
> 
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index 4c43e37..13cc47d 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -111,6 +111,7 @@ struct video_data {
>      int buffers;
>      void **buf_start;
>      unsigned int *buf_len;
> +    int buffers_dequeued;
>      char *standard;
>      v4l2_std_id std_id;
>      int channel;
> @@ -123,6 +124,8 @@ struct video_data {
>  struct buff_data {
>      int index;
>      int fd;
> +    int *buffers_dequeued;
> +    int buffer_copied;
>  };
>  
>  struct fmt_map {
> @@ -466,25 +469,37 @@ static int mmap_init(AVFormatContext *ctx)
>      return 0;
>  }
>  
> -static void mmap_release_buffer(AVPacket *pkt)
> +static int enqueue_buffer(int fd, int index)
>  {
> +    int res;
>      struct v4l2_buffer buf = { 0 };
> -    int res, fd;
> -    struct buff_data *buf_descriptor = pkt->priv;
> -
> -    if (pkt->data == NULL)
> -        return;
>  
> -    buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +    buf.type   = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>      buf.memory = V4L2_MEMORY_MMAP;
> -    buf.index = buf_descriptor->index;
> -    fd = buf_descriptor->fd;
> -    av_free(buf_descriptor);
> +    buf.index  = index;
>  
>      if (v4l2_ioctl(fd, VIDIOC_QBUF, &buf) < 0) {
>          res = AVERROR(errno);
>          av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n", av_err2str(res));
> +        return res;
>      }
> +    return 0;
> +}
> +
> +static void mmap_release_buffer(AVPacket *pkt)
> +{
> +    struct buff_data *buf_descriptor = pkt->priv;
> +
> +    if (pkt->data == NULL)
> +        return;
> +
> +    if (buf_descriptor->buffer_copied) {
> +        av_free(pkt->data);
> +    } else {
> +        if (!enqueue_buffer(buf_descriptor->fd, buf_descriptor->index))

> +            (*buf_descriptor->buffers_dequeued)--;

the deallocation of packets could happen from different thread(s)
so i think this needs either a mutex, an atomic decrement or some
lockless algorithm to achive the same

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130212/0dbeef3a/attachment.asc>