RE: [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core

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

 



On Tue, 27 Nov 2012, Albert Wang wrote:

[snip]

> >> +static int mcam_camera_set_fmt(struct soc_camera_device *icd,
> >> +                    struct v4l2_format *f)
> >> +{
> >> +    struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> >> +    struct mcam_camera *mcam = ici->priv;
> >> +    const struct soc_camera_format_xlate *xlate = NULL;
> >> +    struct v4l2_mbus_framefmt mf;
> >> +    struct v4l2_pix_format *pix = &f->fmt.pix;
> >> +    int ret = 0;
> >
> >No need to initialise ret.
> >
> Yes, but it looks there is no "bad" impact if we initialize it. :)
> I just want to keep the rule: initialize it before use it. :)

No, please, don't. Firstly, it adds bloat. Secondly, such "blind" 
initialisation can hide real bugs: the variable is initialised, so the 
compiler doesn't complain, then you use it in your code, but in reality, 
the value, that you used is meaningless in your context and you get a 
hidden bug.

[snip]

> >> +static int mcam_camera_try_fmt(struct soc_camera_device *icd,
> >> +                    struct v4l2_format *f)
> >> +{
> >> +    struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> >> +    struct mcam_camera *mcam = ici->priv;
> >> +    const struct soc_camera_format_xlate *xlate;
> >> +    struct v4l2_pix_format *pix = &f->fmt.pix;
> >> +    struct v4l2_mbus_framefmt mf;
> >> +    __u32 pixfmt = pix->pixelformat;
> >> +    int ret = 0;
> >
> >No need to initialise ret.
> >
> >> +
> >> +    xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
> >> +    if (!xlate) {
> >> +            cam_err(mcam, "camera: format: %c not found\n",
> >> +                    pix->pixelformat);
> >> +            return -EINVAL;
> >
> >You shouldn't fail .try_fmt() (unless something really bad happens). Just
> >pick up a default supported format.
> >
> Do you means we just need pick up the default supported format when try_fmt()?

If you don't find the requested format - yes, just pick up any format, that 
you can support.

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