On Sat, Mar 04, 2017 at 04:37:43PM -0800, Steve Longerbeam wrote: > > > On 03/04/2017 02:56 AM, Sakari Ailus wrote: > >Hi Steve, > > > >On Fri, Mar 03, 2017 at 02:43:51PM -0800, Steve Longerbeam wrote: > >> > >> > >>On 03/03/2017 03:45 AM, Sakari Ailus wrote: > >>>On Thu, Mar 02, 2017 at 03:07:21PM -0800, Steve Longerbeam wrote: > >>>> > >>>> > >>>>On 03/02/2017 07:53 AM, Sakari Ailus wrote: > >>>>>Hi Steve, > >>>>> > >>>>>On Wed, Feb 15, 2017 at 06:19:15PM -0800, Steve Longerbeam wrote: > >>>>>>Add a new FRAME_TIMEOUT event to signal that a video capture or > >>>>>>output device has timed out waiting for reception or transmit > >>>>>>completion of a video frame. > >>>>>> > >>>>>>Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx> > >>>>>>--- > >>>>>>Documentation/media/uapi/v4l/vidioc-dqevent.rst | 5 +++++ > >>>>>>Documentation/media/videodev2.h.rst.exceptions | 1 + > >>>>>>include/uapi/linux/videodev2.h | 1 + > >>>>>>3 files changed, 7 insertions(+) > >>>>>> > >>>>>>diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst > >>>>>>index 8d663a7..dd77d9b 100644 > >>>>>>--- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst > >>>>>>+++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst > >>>>>>@@ -197,6 +197,11 @@ call. > >>>>>> the regions changes. This event has a struct > >>>>>> :c:type:`v4l2_event_motion_det` > >>>>>> associated with it. > >>>>>>+ * - ``V4L2_EVENT_FRAME_TIMEOUT`` > >>>>>>+ - 7 > >>>>>>+ - This event is triggered when the video capture or output device > >>>>>>+ has timed out waiting for the reception or transmit completion of > >>>>>>+ a frame of video. > >>>>> > >>>>>As you're adding a new interface, I suppose you have an implementation > >>>>>around. How do you determine what that timeout should be? > >>>> > >>>>The imx-media driver sets the timeout to 1 second, or 30 frame > >>>>periods at 30 fps. > >>> > >>>The frame rate is not necessarily constant during streaming. It may well > >>>change as a result of lighting conditions. > >> > >>I think you mean that would be a _temporary_ change in frame rate, but > >>yes I agree the data rate can temporarily fluctuate. Although I doubt > >>lighting conditions would cause a sensor to pause data transmission > >>for a full 1 second. > > > >Likely not, at least not in typical conditions. The exposure time is still > >quite specific to applications: it could be minutes if you take photos e.g. > >of the night sky. > > > >What I'm saying here is that any static value is likely not both reasonable > >and workable in all potential situations all the time. Still there are cases > >(as yours below) that may happen in relatively common cases on some hardware > >(more common than taking long exposure photos of the night sky with the said > >hardware :)). > > I doubt night photography will ever be a use-case for i.MX. The most > common use-case for this driver will be used in automotive applications > such as rear-view or 360 degree view cameras. Ack. > > > > > >> > >> > >>>I wouldn't add an event for this: > >>>this is unreliable and 30 times the frame period is an arbitrary value > >>>anyway. No other drivers do this either. > >> > >>If no other drivers do this I don't mind removing it. It is really meant > >>to deal with the ADV718x CVBS decoder, which often simply stops sending > >>data on the BT.656 bus if there is an interruption in the input analog > >>signal. But I agree that user space could detect this timeout instead. > >>Unless I hear from someone else that they would like to keep this > >>feature I'll remove it in version 5. > > > >That's a bit of a special situation --- still there are alike conditions on > >existing hardware. You should return the buffers to the user with the ERROR > >flag set --- or return -EIO from VIDIOC_DQBUF, if the condition will > >persist: > > On i.MX an EOF timeout is not recoverable without a stream restart, so > I decided to call vb2_queue_error() when the timeout occurs (instead > of sending an event). The user will then get -EIO when it attempts to > queue or dequeue further buffers. It believe that's the correct thing to do. > > > > > ><URL:https://www.linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-qbuf.html> > > > >Do you already obtain the frame rate from the image source (e.g. tuner, > >sensor, decoder) and multiply the frame time by some number in the current > >implementation? > > No the timeout is a constant value, regardless of the source frame > rate. Should the timeout be based on a constant time, or based on a > constant # of frames? I really don't think it matters much, what matters > is that it be long enough to be reasonably sure no data is forthcoming, > for most use-cases. That should be fine. If there is a use case that requires something else, then the implementation can be changed: it's not visible to the user space anyway. -- Kind regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx