Re: [yavta PATCH 5/9] Allow passing file descriptors to yavta

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Thursday 10 April 2014 21:48:55 Sakari Ailus wrote:
> Hi Laurent,
> 
> Thanks for the comments.
> 
> Laurent Pinchart wrote:
> ...
> 
> >> @@ -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() ?
> 
> Agreed. I'll separate queue type selection from querycap. As the
> querycap needs to be done after opening the device, I'll put it into
> another function. I'm ok with video_init(), but what would you think
> about e.g. video_set_queue_type() as the function does nothing else.

Just thinking out loud, we need to

- initialize the device structure,
- open the device or use an externally provided fd,
- optionally query the device capabilities,
- optionally override the queue type.

Initializing the device structure must be performed unconditionally, I would 
create a video_init() function for that.

Opening the device or using an externally provided fd are exclusive 
operations, I would create two functions (that wouldn't do much).

Querying the device capabilities is also optional, I would create one function 
for that.

Finally, overriding the queue type is of course optional and should be 
implemented in its own function. We should probably return an error if the 
user tries to set a queue type not reported by QUERYCAP (assuming QUERYCAP has 
been called).

Ideally I'd also like to make the --no-query argument non-mandatory when 
operating on subdev nodes. It has been introduced because QUERYCAP isn't 
supported by subdev nodes, and it would be nice if we could detect somehow 
that the device node corresponds to a subdev and automatically skip QUERYCAP 
in that case.

-- 
Regards,

Laurent Pinchart