Hi Mauro,
On Sunday 06 March 2011 14:32:44 Mauro Carvalho Chehab wrote:
> Em 06-03-2011 08:38, Laurent Pinchart escreveu:
> > On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
> >> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
> >>
> >> A somewhat unrelated question that occurred to me today: what happens
> >> when a format change happens while streaming?
> >>
> >> Considering that some formats need more bits than others, this could
> >> lead into buffer overflows, either internally at the device or
> >> externally, on bridges that just forward whatever it receives to the
> >> DMA buffers (there are some that just does that). I didn't see anything
> >> inside the mc code preventing such condition to happen, and probably
> >> implementing it won't be an easy job. So, one alternative would be to
> >> require some special CAPS if userspace tries to set the mbus format
> >> directly, or to recommend userspace to create media controller nodes
> >> with 0600 permission.
> >
> > That's not really a media controller issue. Whether formats can be
> > changed during streaming is a driver decision. The OMAP3 ISP driver
> > won't allow formats to be changed during streaming. If the hardware
> > allows for such format changes, drivers can implement support for that
> > and make sure that no buffer overflow will occur.
>
> Such issues is caused by having two API's that allow format changes, one
> that does it device-based, and another one doing it subdev-based.
>
> Ok, drivers can implementing locks to prevent such troubles, but, without
> the core providing a reliable mechanism, it is hard to implement a
> correct lock.
>
> For example, let's suppose that some driver is using mt9m111 subdev (I just
> picked one random sensor that supports lots of MBUS formats). There's
> nothing there preventing a subdev call for it to change mbus format while
> streaming. Worse than that, the sensor driver has no way to block it, as
> it doesn't know that the bridge driver is streaming or not.
>
> The code at subdev_do_ioctl() is just:
>
> case VIDIOC_SUBDEV_S_FMT: {
> struct v4l2_subdev_format *format = arg;
>
> if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> return -EINVAL;
>
> if (format->pad >= sd->entity.num_pads)
> return -EINVAL;
>
> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
> }
>
> So, mc core won't be preventing it.
>
> So, I can't see how such subdev request would be implementing a logic to
> return -EBUSY on those cases.
Drivers can use the media_device graph_mutex to serialize format and stream
management calls. A finer grain locking mechanism implemented in the core
might be better, but we're not stuck without a solution at the moment.
--
Regards,
Laurent Pinchart
--
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
