RE: [PATCH] [media] v4l: soc-camera: add enum-frame-size ioctl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 9 Jan 2011, Qing Xu wrote:

> On Mon, 10 Jan 2011, Qing Xu wrote:
> 
> > On Fri, 7 Jan 2011, Guennadi Liakhovetski wrote:
> 
> > On Fri, 7 Jan 2011, Qing Xu wrote:
> >
> > > pass VIDIOC_ENUM_FRAMESIZES down to sub device drivers. So far no
> > > special handling in soc-camera core.
> >
> > Hm, no, guess what? I don't think this can work. The parameter, that this
> > routine gets from the user struct v4l2_frmsizeenum contains a member
> > pixel_format, which is a fourcc code. Whereas subdevices don't deal with
> > them, they deal with mediabus codes. It is the same problem as with old
> > s/g/try/enum_fmt, which we replaced with respective mbus variants. So, we
> > either have to add our own .enum_mbus_framesizes video subdevice
> > operation, or we abuse this one, but interpret the pixel_format field as a
> > media-bus code.
> >
> > Currently I only see one subdev driver, that implements this API:
> > ov7670.c, and it just happily ignores the pixel_format altogether...
> >
> > Hans, Laurent, what do you think?
> >
> > In the soc-camera case you will have to use soc_camera_xlate_by_fourcc()
> > to convert to media-bus code from fourcc. A nit-pick: please, follow the
> > style of the file, that you patch and don't add double empty lines between
> > functions.
> >
> > A side question: why do you need this format at all? Is it for some custom
> 
> > Sorry, I meant to ask - what do you need this operation / ioctl() for?
> 
> Hi Guennadi,
> 
> Before we launch camera application, we will use enum-frame-size ioctl 
> to get all frame size that the sensor supports, and show all options in 
> UI menu, then the customers could choose a size, and tell camera driver.

And if the camera supports ranges of sizes? Or doesn't implement this 
ioctl() at all? Remember, that this is an optional ioctl(). Would your 
application just fail? Or you could provide a slider to let the user 
select a size from a range, then just issue an s_fmt and use whatever it 
returns... This is something you'd do for a generic application

> If use mbus structure pass to sensor, we need to modify the second 
> parameter definition, it will contain both mbus code information and 
> width/height ingotmation:
> int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize);
> 
> or use g_mbus_fmt instead:
> int (*g_mbus_fmt)(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *fmt);
> soc_camera_enum_framesizes()
> {
>         ...
>         return v4l2_subdev_call(sd, video, g_mbus_fmt, fsize);//typo, I mean "g_mbus_fmt"
> }
> 
> What do you think?

In any case therer needs to be a possibility for host drivers to override 
this routine, so, please, do something similar, to what default_g_crop() / 
default_s_crop() / default_cropcap() / default_g_parm() / default_s_parm() 
do: add a host operation and provide a default implementation for it. And 
since noone seems to care enough, I think, we can just abuse struct 
v4l2_frmsizeenum for now, just make sure to rewrite pixel_format to a 
media-bus code, and restore it before returning to the caller.

Thanks
Guennadi

> Thanks!
> Qing
> 
> > application? What is your use-case, that makes try_fmt / s_fmt
> > insufficient for you and how does enum_framesizes help you?
> >
> > Thanks
> > Guennadi
> >
> > >
> > > Signed-off-by: Kassey Lee <ygli@xxxxxxxxxxx>
> > > Signed-off-by: Qing Xu <qingx@xxxxxxxxxxx>
> > > ---
> > >  drivers/media/video/soc_camera.c |   11 +++++++++++
> > >  1 files changed, 11 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> > > index 052bd6d..11715fb 100644
> > > --- a/drivers/media/video/soc_camera.c
> > > +++ b/drivers/media/video/soc_camera.c
> > > @@ -145,6 +145,16 @@ static int soc_camera_s_std(struct file *file, void *priv, v4l2_std_id *a)
> > >     return v4l2_subdev_call(sd, core, s_std, *a);
> > >  }
> > >
> > > +static int soc_camera_enum_framesizes(struct file *file, void *fh,
> > > +                                    struct v4l2_frmsizeenum *fsize)
> > > +{
> > > +   struct soc_camera_device *icd = file->private_data;
> > > +   struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> > > +
> > > +   return v4l2_subdev_call(sd, video, enum_framesizes, fsize);
> > > +}
> > > +
> > > +
> > >  static int soc_camera_reqbufs(struct file *file, void *priv,
> > >                           struct v4l2_requestbuffers *p)
> > >  {
> > > @@ -1302,6 +1312,7 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = {
> > >     .vidioc_g_input          = soc_camera_g_input,
> > >     .vidioc_s_input          = soc_camera_s_input,
> > >     .vidioc_s_std            = soc_camera_s_std,
> > > +   .vidioc_enum_framesizes  = soc_camera_enum_framesizes,
> > >     .vidioc_reqbufs          = soc_camera_reqbufs,
> > >     .vidioc_try_fmt_vid_cap  = soc_camera_try_fmt_vid_cap,
> > >     .vidioc_querybuf         = soc_camera_querybuf,
> > > --
> > > 1.6.3.3
> > >
> >
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux