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
