Hi Sakari,

Thank you for the patch.

On Saturday 01 March 2014 18:18:06 Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ai...@iki.fi>
> ---
>  yavta.c |   63 ++++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/yavta.c b/yavta.c
> index 870682e..a9b192a 100644
> --- a/yavta.c
> +++ b/yavta.c
> @@ -62,6 +62,7 @@ struct buffer
>  struct device
>  {
>       int fd;
> +     int opened;
> 
>       enum v4l2_buf_type type;
>       enum v4l2_memory memtype;
> @@ -180,13 +181,8 @@ static unsigned int v4l2_format_code(const char *name)
>       return 0;
>  }
> 
> -static int video_open(struct device *dev, const char *devname, int
> no_query)
> +static int video_open(struct device *dev, const char *devname)
>  {
> -     dev->fd = -1;
> -     dev->memtype = V4L2_MEMORY_MMAP;
> -     dev->buffers = NULL;
> -     dev->type = (enum v4l2_buf_type)-1;
> -
>       dev->fd = open(devname, O_RDWR);
>       if (dev->fd < 0) {
>               printf("Error opening device %s: %s (%d).\n", devname,
> @@ -196,6 +192,16 @@ static int video_open(struct device *dev, const char
> *devname, int no_query)
> 
>       printf("Device %s opened.\n", devname);
> 
> +     dev->opened = 1;
> +
> +     return 0;
> +}
> +
> +static int video_querycap(struct device *dev, int no_query) {
> +     struct v4l2_capability cap;
> +     unsigned int capabilities;
> +     int ret;
> +

video_querycap ends up setting the dev->type field, which isn't really the job 
of a query function. Would there be a clean way to pass the fd to the 
video_open() function instead ? Maybe video_open() could be split and/or 
renamed to video_init() ?

>       if (no_query) {
>               /* Assume capture device. */
>               dev->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> @@ -215,9 +221,7 @@ static int video_open(struct device *dev, const char
> *devname, int no_query) else if (capabilities & V4L2_CAP_VIDEO_OUTPUT)
>               dev->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>       else {
> -             printf("Error opening device %s: neither video capture "
> -                     "nor video output supported.\n", devname);
> -             close(dev->fd);
> +             printf("Device supports neither capture nor output.\n");
>               return -EINVAL;
>       }
> 
> @@ -231,7 +235,8 @@ static void video_close(struct device *dev)
>  {
>       free(dev->pattern);
>       free(dev->buffers);
> -     close(dev->fd);
> +     if (dev->opened)
> +             close(dev->fd);
>  }
> 
>  static unsigned int get_control_type(struct device *dev, unsigned int id)
> @@ -1246,6 +1251,7 @@ static void usage(const char *argv0)
>       printf("-w, --set-control 'ctrl value'  Set control 'ctrl' to 
'value'\n");
>       printf("    --enum-formats              Enumerate formats\n");
>       printf("    --enum-inputs               Enumerate inputs\n");
> +     printf("    --fd                        Use a numeric file descriptor
> insted of a device\n");
>       printf("    --no-query                  Don't query capabilities on 
> open\n");
>       printf("    --offset                    User pointer buffer offset from 
> page
> start\n");
>       printf("    --requeue-last              Requeue the last buffers before
> streamoff\n");
> @@ -1262,6 +1268,7 @@ static void usage(const char *argv0)
>  #define OPT_USERPTR_OFFSET   261
>  #define OPT_REQUEUE_LAST     262
>  #define OPT_STRIDE           263
> +#define OPT_FD                       264
> 
>  static struct option opts[] = {
>       {"capture", 2, 0, 'c'},
> @@ -1269,6 +1276,7 @@ static struct option opts[] = {
>       {"delay", 1, 0, 'd'},
>       {"enum-formats", 0, 0, OPT_ENUM_FORMATS},
>       {"enum-inputs", 0, 0, OPT_ENUM_INPUTS},
> +     {"fd", 1, 0, OPT_FD},
>       {"file", 2, 0, 'F'},
>       {"fill-frames", 0, 0, 'I'},
>       {"format", 1, 0, 'f'},
> @@ -1297,7 +1305,11 @@ static struct option opts[] = {
>  int main(int argc, char *argv[])
>  {
>       struct sched_param sched;
> -     struct device dev = { 0 };
> +     struct device dev = {
> +             .fd = -1,
> +             .memtype = V4L2_MEMORY_MMAP,
> +             .type = (enum v4l2_buf_type)-1,
> +     };
>       int ret;
> 
>       /* Options parsings */
> @@ -1452,6 +1464,14 @@ int main(int argc, char *argv[])
>               case OPT_ENUM_INPUTS:
>                       do_enum_inputs = 1;
>                       break;
> +             case OPT_FD:
> +                     dev.fd = atoi(optarg);
> +                     if (dev.fd < 0) {
> +                             printf("Bad file descriptor %d\n", dev.fd);
> +                             return 1;
> +                     }
> +                     printf("Using file descriptor %d\n", dev.fd);
> +                     break;
>               case OPT_NO_QUERY:
>                       no_query = 1;
>                       break;
> @@ -1482,18 +1502,21 @@ int main(int argc, char *argv[])
>               return 1;
>       }
> 
> -     if (optind >= argc) {
> -             usage(argv[0]);
> -             return 1;
> -     }
> -
>       if (!do_file)
>               filename = NULL;
> 
> -     /* Open the video device. If the device type isn't recognized, set the
> -      * --no-query option to avoid querying V4L2 subdevs.
> -      */
> -     ret = video_open(&dev, argv[optind], no_query);
> +     if (dev.fd == -1) {
> +             if (optind >= argc) {
> +                     usage(argv[0]);
> +                     return 1;
> +             } else {
> +                     ret = video_open(&dev, argv[optind]);
> +                     if (ret < 0)
> +                             return 1;
> +             }
> +     }
> +
> +     ret = video_querycap(&dev, no_query);
>       if (ret < 0)
>               return 1;

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to