Announcing libv4l 0.3.7

Announcing libv4l 0.3.7

[Hans de Goede]

Hi All,

Here is release 0.3.7 nothing special really, just adding a few more cam 
specific formats to libv4lconvert.

This release has the following changes:

libv4l-0.3.7
------------
* Add spca505/6 and spca508 cam specific formats (YUYV per line variations)

Go grab it here:
http://people.atrpms.net/~hdegoede/libv4l-0.3.7.tar.gz


Regards,

Hans

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Messed up syscall return value

[Gregor Jasny]

[-- Attachment #1: Type: text/plain, Size: 5514 bytes --]

Hi,

I've observed strange behavior in the REQBUFS ioctl for the non-emulated
case. To reproduce:
* Use a amd64 system (Debian Sid if it matters)
* Linux 2.6.26
* load vivi driver to video0
* Compile attached program w/o libv4l2 and check that it runs fine.
* Compile attached program with libv4l2 and see it fail during REQBUFS
  ioctl.

I've debugged the problem to the line:
result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, req);

Here the value 2 get stored into result, although the kernel driver
returned success (at least it does not complain loudly in the logs).

When stepping assembly instructions the rax value is zero, shortly after
it is set to two. Personally I suspect a messed up stack. Both
v4l2_ioctl and syscall are variable argument functions.

Thanks,
Gregor

Kernel log for pure V4L2;
Jul 28 23:20:09 Rincewind kernel: vivi: open called (minor=0)
Jul 28 23:20:09 Rincewind kernel: vivi: open minor=0 type=video-cap users=1
Jul 28 23:20:09 Rincewind kernel: vivi: vivi_start_thread
Jul 28 23:20:09 Rincewind kernel: vivi: returning from vivi_start_thread
Jul 28 23:20:09 Rincewind kernel: vivi: thread started
Jul 28 23:20:09 Rincewind kernel: vivi: vivi_sleep dma_q=0xffff81005c480640
Jul 28 23:20:09 Rincewind kernel: vivi: Thread tick
Jul 28 23:20:09 Rincewind kernel: vivi: No active queue to serve
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_S_FMT
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_REQBUFS
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_setup, count=2, size=202752
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_QUERYBUF
Jul 28 23:20:09 Rincewind kernel: vivi: mmap called, vma=0xffff810068086690
Jul 28 23:20:09 Rincewind kernel: vivi: vma start=0x7f8734c0a000, size=204800, ret=0
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_QUERYBUF
Jul 28 23:20:09 Rincewind kernel: vivi: mmap called, vma=0xffff8100680863f0
Jul 28 23:20:09 Rincewind kernel: vivi: vma start=0x7f8734bd8000, size=204800, ret=0
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_QBUF
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_prepare, field=4
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_QBUF
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_prepare, field=4
Jul 28 23:20:09 Rincewind kernel: vivi (0): VIDIOC_STREAMON
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_queue
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_queue
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_release
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer, state: 5
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer: freed
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_release
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer, state: 5
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer: freed
Jul 28 23:20:09 Rincewind kernel: vivi: vivi_stop_thread
Jul 28 23:20:09 Rincewind kernel: vivi: thread: exit
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_release
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer, state: 0
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer: freed
Jul 28 23:20:09 Rincewind kernel: vivi: buffer_release
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer, state: 0
Jul 28 23:20:09 Rincewind kernel: vivi: free_buffer: freed
Jul 28 23:20:09 Rincewind kernel: vivi: close called (minor=0, users=0)

Kernel Log for libv4l2 case:
Jul 28 23:21:03 Rincewind kernel: vivi: open called (minor=0)
Jul 28 23:21:03 Rincewind kernel: vivi: open minor=0 type=video-cap users=1
Jul 28 23:21:03 Rincewind kernel: vivi: vivi_start_thread
Jul 28 23:21:03 Rincewind kernel: vivi: returning from vivi_start_thread
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_QUERYCAP
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_G_FMT
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_ENUM_FMT
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_ENUM_FMT
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_TRY_FMT
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_S_FMT
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_REQBUFS
Jul 28 23:21:03 Rincewind kernel: vivi: thread started
Jul 28 23:21:03 Rincewind kernel: vivi: buffer_setup, count=2, size=202752
Jul 28 23:21:03 Rincewind kernel: vivi: vivi_sleep dma_q=0xffff81005c480640
Jul 28 23:21:03 Rincewind kernel: vivi: Thread tick
Jul 28 23:21:03 Rincewind kernel: vivi: No active queue to serve
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_QUERYBUF
Jul 28 23:21:03 Rincewind kernel: vivi: mmap called, vma=0xffff81007ab7ea80
Jul 28 23:21:03 Rincewind kernel: vivi: vma start=0x7f29aa02f000, size=204800, ret=0
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_QUERYBUF
Jul 28 23:21:03 Rincewind kernel: vivi: mmap called, vma=0xffff810070e260a8
Jul 28 23:21:03 Rincewind kernel: vivi: vma start=0x7f29a9ffd000, size=204800, ret=0
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_QBUF
Jul 28 23:21:03 Rincewind kernel: vivi: buffer_prepare, field=4
Jul 28 23:21:03 Rincewind kernel: vivi (0): VIDIOC_QBUF
Jul 28 23:21:03 Rincewind kernel: vivi: buffer_prepare, field=4
Jul 28 23:21:03 Rincewind kernel: vivi: vivi_stop_thread
Jul 28 23:21:03 Rincewind kernel: vivi: thread: exit
Jul 28 23:21:03 Rincewind kernel: vivi: buffer_release
Jul 28 23:21:03 Rincewind kernel: vivi: free_buffer, state: 1
Jul 28 23:21:03 Rincewind kernel: vivi: free_buffer: freed
Jul 28 23:21:03 Rincewind kernel: vivi: buffer_release
Jul 28 23:21:03 Rincewind kernel: vivi: free_buffer, state: 1
Jul 28 23:21:03 Rincewind kernel: vivi: free_buffer: freed
Jul 28 23:21:03 Rincewind kernel: vivi: close called (minor=0, users=0)


[-- Attachment #2: reqbufs.c --]
[-- Type: text/x-csrc, Size: 3765 bytes --]

// gcc -std=gnu99 -ggdb -L /home/gjasny/src/libv4l-0.3.7/libv4l2 -L /home/gjasny/src/libv4l-0.3.7/libv4lconvert -I/home/gjasny/src/libv4l-0.3.7/include -lv4l2 -lv4lconvert -o reqbufs reqbufs.c

#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <linux/ioctl.h>
#include <linux/videodev2.h>

#if 1
# include <libv4l2.h>
# define my_ioctl       v4l2_ioctl
# define my_open        v4l2_open
# define my_close       v4l2_close
# define my_mmap        v4l2_mmap
#else
# define my_ioctl       ioctl
# define my_open        open
# define my_close       close
# define my_mmap        mmap
#endif

#define NBUFFERS        2
#define WIDTH           352
#define HEIGHT          288

struct buffer {
    void   *start;
    size_t length;
} g_buffers[NBUFFERS];

static int xioctl( int fd, unsigned long int request, void *arg )
{
    int r;

    do r = my_ioctl (fd, request, arg);
    while (r == -1 && errno == EINTR);

    return r;
}

int main(int argc, char *argv[])
{
    const char *device = "/dev/video0";

    int fd = my_open (device, O_RDWR);
    if (!fd) {
        perror ("open");
        return -1;
    }
    
    struct v4l2_format format;
    memset( &format, 0, sizeof(struct v4l2_format));
    format.type                = V4L2_BUF_TYPE_VIDEO_CAPTURE;
    format.fmt.pix.width       = WIDTH;
    format.fmt.pix.height      = HEIGHT;
    format.fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
    format.fmt.pix.field       = V4L2_FIELD_ANY;

    if (xioctl (fd, VIDIOC_S_FMT, &format) == -1) {
        perror("ioctl (VIDIOC_S_FMT)");
        return -1;
    }

    // init mmap
    struct v4l2_requestbuffers m_req;
    memset( &m_req, 0, sizeof(struct v4l2_requestbuffers) );

    m_req.count   = NBUFFERS;
    m_req.type    = V4L2_BUF_TYPE_VIDEO_CAPTURE;
    m_req.memory  = V4L2_MEMORY_MMAP;

    if (xioctl (fd, VIDIOC_REQBUFS, &m_req) == -1) {
        if (EINVAL == errno) {
            fprintf (stderr, "device does not support memory mapping.\n");
        } else {
            perror ("ioctl (VIDIOC_REQBUFS)");
        }
        return -1;
    }

    if (m_req.count < NBUFFERS) {
        fprintf (stderr, "Insufficient buffer memory.\n");
        return -1;
    }

    for (unsigned i = 0; i < m_req.count; ++i) {

        struct v4l2_buffer buf;
        memset( &buf, 0, sizeof(buf) );

        buf.type        = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        buf.memory      = V4L2_MEMORY_MMAP;
        buf.index       = i;

        if (xioctl (fd, VIDIOC_QUERYBUF, &buf) == -1) {
            perror("ioctl (VIDIOC_QUERYBUF)");
            return -1;
        }

        g_buffers[i].length = buf.length;
        g_buffers[i].start  = my_mmap (NULL /* start anywhere */,
                                       buf.length,
                                       PROT_READ | PROT_WRITE /* required */,
                                       MAP_SHARED /* recommended */,
                                       fd, buf.m.offset);

        if (g_buffers[i].start == MAP_FAILED) {
            perror("mmap");
            return -1;
        }
    }

    for (unsigned i = 0; i < NBUFFERS; ++i) {
        struct v4l2_buffer buf;
        memset( &buf, 0, sizeof(buf) );

        buf.type        = V4L2_BUF_TYPE_VIDEO_CAPTURE;
        buf.memory      = V4L2_MEMORY_MMAP;
        buf.index       = i;

        if (xioctl (fd, VIDIOC_QBUF, &buf) == -1) {
            perror ("ioctl (VIDIOC_QBUF)");
            return -1;
        }
    }

    enum v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
    if (xioctl (fd, VIDIOC_STREAMON, &type) == -1) {
        perror("ioctl (VIDIOC_STREAMON)");
        return -1;
    }

    my_close (fd);

    return -1;
}

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Gregor Jasny]

On Mon, Jul 28, 2008 at 11:56:32PM +0200, Jiri Slaby wrote:
> On 07/28/2008 11:49 PM, Gregor Jasny wrote:
> >I've observed strange behavior in the REQBUFS ioctl for the non-emulated
> >case.
> >
> >I've debugged the problem to the line:
> >result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, req);
> >
> >Here the value 2 get stored into result, although the kernel driver
> >returned success (at least it does not complain loudly in the logs).
> >
> strace and ltrace will help in this case I guess. Could you provide them for
> your testcases?

Sure:

ioctl(3, VIDIOC_QUERYCAP or VT_OPENQRY, 0x7fffbfd9fdd0) = 0
ioctl(3, VIDIOC_G_FMT or VT_SENDSIG, 0x7fffbfd9fd00) = 0
ioctl(3, VIDIOC_ENUM_FMT or VT_SETMODE, 0x7fffbfd9fc40) = 0
ioctl(3, VIDIOC_ENUM_FMT or VT_SETMODE, 0x7fffbfd9fc40) = -1 EINVAL (Invalid argument)
ioctl(3, VIDIOC_TRY_FMT, 0x7fffbfda0080) = 0
ioctl(3, VIDIOC_S_FMT or VT_RELDISP, 0x7fffbfd9fd40) = 0
ioctl(3, VIDIOC_REQBUFS or VT_DISALLOCATE, 0x7fffbfda0060) = 2

Huh? Something evils seems to be going on in V4L2 land.
I've spotted the following lines in videobuf-core.c:videobuf_reqbufs

        req->count = retval;

 done:
        mutex_unlock(&q->vb_lock);
        return retval;

That would explain the retval '2'. It seems a retval = 0; statement is missing here for the success case.

Gregor

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Gregor Jasny]

On Tue, Jul 29, 2008 at 12:22:52AM +0200, Jiri Slaby wrote:
> Actually positive ioctl retval used to be often considered as OK in the past
> (and this approach is still used in few char drivers).
> 
> But according to v4l docco, it isn't permitted here. Anyway I wouldn't 
> place it
> in videobuf-core.c, but in vivi code; letting this decision on Mauro (CCed) 
> ;).

What happens to negative retvals like -EINVAL? Get they somewhere truncated to -1 and
errno set?

Just curious,
Gregor

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[H. Willstrand]

Hi!

I think the memory allocation is wrong, you have NBUFFERS = 2 but
memset( ... ) only allocates for 1 buffer.

Regards,
H.Willstrand

On Tue, 2008-07-29 at 00:22 +0200, Jiri Slaby wrote:
> On 07/29/2008 12:16 AM, Gregor Jasny wrote:
> > ioctl(3, VIDIOC_REQBUFS or VT_DISALLOCATE, 0x7fffbfda0060) = 2
> > 
> > Huh? Something evils seems to be going on in V4L2 land.
> > I've spotted the following lines in videobuf-core.c:videobuf_reqbufs
> > 
> >         req->count = retval;
> > 
> >  done:
> >         mutex_unlock(&q->vb_lock);
> >         return retval;
> > 
> > That would explain the retval '2'. It seems a retval = 0; statement is missing here for the success case.
> 
> Actually positive ioctl retval used to be often considered as OK in the past 
> (and this approach is still used in few char drivers).
> 
> But according to v4l docco, it isn't permitted here. Anyway I wouldn't place it 
> in videobuf-core.c, but in vivi code; letting this decision on Mauro (CCed) ;).
> 
> _______________________________________________
> V4L2-library mailing list
> V4L2-library@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l2-library

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Hans de Goede]

Jiri Slaby wrote:
> On 07/29/2008 12:16 AM, Gregor Jasny wrote:
>> ioctl(3, VIDIOC_REQBUFS or VT_DISALLOCATE, 0x7fffbfda0060) = 2
>>
>> Huh? Something evils seems to be going on in V4L2 land.
>> I've spotted the following lines in videobuf-core.c:videobuf_reqbufs
>>
>>         req->count = retval;
>>
>>  done:
>>         mutex_unlock(&q->vb_lock);
>>         return retval;
>>
>> That would explain the retval '2'. It seems a retval = 0; statement is 
>> missing here for the success case.
>

Indeed, so iow I'm happy to conclude that thie is not a libv4l bug :)

Regards,

Hans

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Gregor Jasny]

On Tue, Jul 29, 2008 at 12:00:34PM +0200, Hans de Goede wrote:
> Indeed, so iow I'm happy to conclude that thie is not a libv4l bug :)

Will you add a work around like the above in libv4l?

Thanks,
Gregor

Index: libv4l2/libv4l2.c
===================================================================
RCS file: /var/cvs/vidsoft/extern/libv4l/libv4l2/libv4l2.c,v
retrieving revision 1.5
diff -u -r1.5 libv4l2.c
--- libv4l2/libv4l2.c	14 Jul 2008 14:00:28 -0000	1.5
+++ libv4l2/libv4l2.c	29 Jul 2008 11:51:20 -0000
@@ -97,7 +97,7 @@
   req.count = devices[index].nreadbuffers;
   req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   req.memory = V4L2_MEMORY_MMAP;
-  if ((result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, &req))){
+  if ((result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, &req)) < 0){
     int saved_err = errno;
     V4L2_LOG_ERR("requesting buffers: %s\n", strerror(errno));
     errno = saved_err;
@@ -121,7 +121,7 @@
   req.count = 0;
   req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   req.memory = V4L2_MEMORY_MMAP;
-  if ((result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, &req))) {
+  if ((result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, &req)) < 0) {
     int saved_err = errno;
     V4L2_LOG_ERR("unrequesting buffers: %s\n", strerror(errno));
     errno = saved_err;
@@ -723,8 +723,9 @@
 	v4l2_unmap_buffers(index);
 
 	result = syscall(SYS_ioctl, devices[index].fd, VIDIOC_REQBUFS, req);
-	if (result)
+	if (result < 0)
 	  break;
+	result = 0; // some drivers return the number of buffers on success
 
 	/* If we got more frames then we can handle lie to the app */
 	if (req->count > V4L2_MAX_NO_FRAMES)

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Hans de Goede]

Gregor Jasny wrote:
> On Tue, Jul 29, 2008 at 12:00:34PM +0200, Hans de Goede wrote:
>> Indeed, so iow I'm happy to conclude that thie is not a libv4l bug :)
> 
> Will you add a work around like the above in libv4l?
> 

I don't like it much, but since Mauro thinks this is for the best I've added to 
my tree.

Regards,

Hans

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Mauro Carvalho Chehab]

On Tue, 29 Jul 2008 00:22:52 +0200
Jiri Slaby <jirislaby@gmail.com> wrote:

> On 07/29/2008 12:16 AM, Gregor Jasny wrote:
> > ioctl(3, VIDIOC_REQBUFS or VT_DISALLOCATE, 0x7fffbfda0060) = 2
> > 
> > Huh? Something evils seems to be going on in V4L2 land.
> > I've spotted the following lines in videobuf-core.c:videobuf_reqbufs
> > 
> >         req->count = retval;
> > 
> >  done:
> >         mutex_unlock(&q->vb_lock);
> >         return retval;
> > 
> > That would explain the retval '2'. It seems a retval = 0; statement is missing here for the success case.
> 
> Actually positive ioctl retval used to be often considered as OK in the past 
> (and this approach is still used in few char drivers).
> 
> But according to v4l docco, it isn't permitted here. Anyway I wouldn't place it 
> in videobuf-core.c, but in vivi code; letting this decision on Mauro (CCed) ;).

This is what videobuf-core do, if success:

        req->count = retval;

 done:
        mutex_unlock(&q->vb_lock);
        return retval;

So, it returns the number of buffers that were really allocated. It is too late
to change this, since this behaviour is very old. If the V4L2 API spec is
different, we should fix at the spec, not at the driver.

IMO, the library patch proposed should be applied. All error checks should test
for values lower than zero, since positive values don't indicate errors.

Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Laurent Pinchart]

On Tuesday 29 July 2008, Mauro Carvalho Chehab wrote:
> On Tue, 29 Jul 2008 00:22:52 +0200
>
> Jiri Slaby <jirislaby@gmail.com> wrote:
> > On 07/29/2008 12:16 AM, Gregor Jasny wrote:
> > > ioctl(3, VIDIOC_REQBUFS or VT_DISALLOCATE, 0x7fffbfda0060) = 2
> > >
> > > Huh? Something evils seems to be going on in V4L2 land.
> > > I've spotted the following lines in videobuf-core.c:videobuf_reqbufs
> > >
> > >         req->count = retval;
> > >
> > >  done:
> > >         mutex_unlock(&q->vb_lock);
> > >         return retval;
> > >
> > > That would explain the retval '2'. It seems a retval = 0; statement is
> > > missing here for the success case.
> >
> > Actually positive ioctl retval used to be often considered as OK in the
> > past (and this approach is still used in few char drivers).
> >
> > But according to v4l docco, it isn't permitted here. Anyway I wouldn't
> > place it in videobuf-core.c, but in vivi code; letting this decision on
> > Mauro (CCed) ;).
>
> This is what videobuf-core do, if success:
>
>         req->count = retval;
>
>  done:
>         mutex_unlock(&q->vb_lock);
>         return retval;
>
> So, it returns the number of buffers that were really allocated. It is too
> late to change this, since this behaviour is very old. If the V4L2 API spec
> is different, we should fix at the spec, not at the driver.

I'm not sure to agree with that. The spec clearly states that 0 is returned on 
success and -1 on error. Since applications don't choke with the currently 
buggy videobuf-core implementation, they must all be check against -1, or 
checking for a negative error code. So returning 0 shouldn't break any 
application, except if it relies on the ioctl returning the number of 
buffers, which is not documented anywhere.

> IMO, the library patch proposed should be applied. All error checks should
> test for values lower than zero, since positive values don't indicate
> errors.

Best regards,

Laurent Pinchart

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Mauro Carvalho Chehab]

On Tue, 29 Jul 2008 21:05:17 +0200
Laurent Pinchart <laurent.pinchart@skynet.be> wrote:

> > So, it returns the number of buffers that were really allocated. It is too
> > late to change this, since this behaviour is very old. If the V4L2 API spec
> > is different, we should fix at the spec, not at the driver.
> 
> I'm not sure to agree with that. The spec clearly states that 0 is returned on 
> success and -1 on error. Since applications don't choke with the currently 
> buggy videobuf-core implementation, they must all be check against -1, or 
> checking for a negative error code. So returning 0 shouldn't break any 
> application, except if it relies on the ioctl returning the number of 
> buffers, which is not documented anywhere.

There are some apps that relies on the number of buffers allocated. This is
important on some cases. For example, if you have digital audio support,
userspace app needs to know how much buffers were allocated, in order to proper
synchronize audio and video.

In the case of videobuf, used by most drivers since pre-2.6 kernels, it returns
it at both req->count and as the returned value for ioctl.

So, userspace apps may use req->count to know the effective number of allocated
buffers, or the returned value for the ioctl. Since most apps were done
considering bttv (a client of videobuf), I don't doubt that some of them are
just using the returned value at the ioctl.

Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [V4l2-library] Messed up syscall return value

[Laurent Pinchart]

On Tuesday 29 July 2008, Mauro Carvalho Chehab wrote:
> On Tue, 29 Jul 2008 21:05:17 +0200
>
> Laurent Pinchart <laurent.pinchart@skynet.be> wrote:
> > > So, it returns the number of buffers that were really allocated. It is
> > > too late to change this, since this behaviour is very old. If the V4L2
> > > API spec is different, we should fix at the spec, not at the driver.
> >
> > I'm not sure to agree with that. The spec clearly states that 0 is
> > returned on success and -1 on error. Since applications don't choke with
> > the currently buggy videobuf-core implementation, they must all be check
> > against -1, or checking for a negative error code. So returning 0
> > shouldn't break any application, except if it relies on the ioctl
> > returning the number of buffers, which is not documented anywhere.
>
> There are some apps that relies on the number of buffers allocated. This is
> important on some cases. For example, if you have digital audio support,
> userspace app needs to know how much buffers were allocated, in order to
> proper synchronize audio and video.
>
> In the case of videobuf, used by most drivers since pre-2.6 kernels, it
> returns it at both req->count and as the returned value for ioctl.
>
> So, userspace apps may use req->count to know the effective number of
> allocated buffers, or the returned value for the ioctl. Since most apps
> were done considering bttv (a client of videobuf), I don't doubt that some
> of them are just using the returned value at the ioctl.

The documentation clearly states that the ioctl returns 0 on success, not the 
number of allocated buffers. Applications that have been developed from the 
spec will not break. Applications that rely on the wrong return value have 
been developed in violation of the spec.

This situation is analogous to using a structure with a reserved field that is 
clearly marked as "don't touch" by the spec. If an application relies on the 
reserved field being set to a specific value, you will surely not refrain 
from changing that value in the kernel because it would break the 
application.

Best regards,

Laurent Pinchart

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list