Re: [PATCH 4/4] v4l2: add blackfin capture bridge driver

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

 



On Fri, 16 Sep 2011, Scott Jiang wrote:

> 2011/9/16 Sylwester Nawrocki <snjw23@xxxxxxxxx>:
> > On 09/15/2011 04:40 AM, Scott Jiang wrote:
> >> 2011/9/14 Sylwester Nawrocki<s.nawrocki@xxxxxxxxxxx>:
> >>> On 09/14/2011 09:10 AM, Scott Jiang wrote:
> >>>>
> >>>>>> +                     fmt =&bcap_formats[i];
> >>>>>> +                     if (mbus_code)
> >>>>>> +                             *mbus_code = fmt->mbus_code;
> >>>>>> +                     if (bpp)
> >>>>>> +                             *bpp = fmt->bpp;
> >>>>>> +                     v4l2_fill_mbus_format(&mbus_fmt, pixfmt,
> >>>>>> +                                             fmt->mbus_code);
> >>>>>> +                     ret = v4l2_subdev_call(bcap->sd, video,
> >>>>>> +                                             try_mbus_fmt,&mbus_fmt);
> >>>>>> +                     if (ret<    0)
> >>>>>> +                             return ret;
> >>>>>> +                     v4l2_fill_pix_format(pixfmt,&mbus_fmt);
> >>>>>> +                     pixfmt->bytesperline = pixfmt->width * fmt->bpp;
> >>>>>> +                     pixfmt->sizeimage = pixfmt->bytesperline
> >>>>>> +                                             * pixfmt->height;
> >
> > No need to clamp mbus_fmt.width and mbus_fmt.height to some maximum values
> > to prevent allocating huge memory buffers ?
> >
> >>>>>
> >>>>> Still pixfmt->pixelformat isn't filled.
> >>>>>
> >>>> no here pixfmt->pixelformat is passed in
> >>>>
> >>>>>> +                     return 0;
> >>>>>> +             }
> >>>>>> +     }
> >>>>>> +     return -EINVAL;
> >>>>>
> >>>>> I think you should return some default format, rather than giving up
> >>>>> when the fourcc doesn't match. However I'm not 100% sure this is
> >>>>> the specification requirement.
> >>>>>
> >>>> no, there is no default format for bridge driver because it knows
> >>>> nothing about this.
> >>>> all the format info bridge needs ask subdevice.
> >>>
> >>> It's the bridge driver that exports a device node and is responsible for
> >>> setting the default format. It should be possible to start streaming right
> >>> after opening the device, without VIDIOC_S_FMT, with some reasonable defaults.
> >>>
> >>> If, as you say, the bridge knows nothing about formats what the bcap_formats[]
> >>> array is here for ?
> >>>
> >> accually this array is to convert mbus to pixformat. ppi supports any formats.
> >> Ideally it should contain all formats in v4l2, but it is enough at
> >> present for our platform.
> >> If I find someone needs more, I will add it.
> >> So return -EINVAL means this format is out of range, it can't be supported now.
> >
> > Ok, fair enough. I guess you rely on subdev driver to return some supported
> > value through mbus_try_fmt and then the bridge driver must be able to handle
> > this. However it might make sense to validate the resolution in some places
> > to prevent allocating insanely huge buffers, when the sensor subdev misbehaves.
> >
> all the format info is got from sensor, so it isn't out of control
> 
> >>
> >> about default format, I think I can only call bcap_g_fmt_vid_cap in
> >> probe to get this info.
> >> Dose anybody have a better solution?
> >
> > How about doing that when device is opened for the first time ?
> >
> no, different input and std has different default format, so I think
> there is no default format is a good choice.
> app should negotiate format before use. I'm not sure all the v4l2 app
> will do this step.
> v4l2 spec only says driver must implement xx ioctl, but it doesn't say
> app must call xx ioctl.
> Anyone can tell me how many steps app must call?

IIRC, the user shall be able to open a v4l2 device, queue buffers and 
start streaming - without setting any format.

> > However it
> > could make more sense to try to set format at the subdev and then check how
> > it was adjusted. Not all subdevs might implement g_mbus_fmt op or some might
> > not deliver sane default values.
> >
> in try_format and s_fmt I have implemented this in bridge driver and
> all my sensor drivers have implemented relative callback.

Thanks
Guennadi
---
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