[Bug 745441] v4l2: Detect lossed frame and warn

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=745441

--- Comment #18 from Peter Seiderer <ps.report at gmx.net> ---
Hello Nicolas,

thanks for reveiew...

(In reply to Nicolas Dufresne (stormer) from comment #17)
> Review of attachment 300279 [details] [review]:
> 
> ::: sys/v4l2/gstv4l2src.c
> @@ +815,3 @@
> +  } else {
> +    /* check for frame loss with given (from v4l2 device) buffer offset */
> +    if ((GST_BUFFER_OFFSET (*buf) != (v4l2src->offset + 1)) &&
> (v4l2src->offset != 0)) {
> 
> Could you use OFFSET_END to avoid the +1 ? And maybe check offset != 0
> before ?

The diff (inclusive OFFSET_END usage) would look like the following:

   } else {
     /* check for frame loss with given (from v4l2 device) buffer offset */
-    if ((GST_BUFFER_OFFSET (*buf) != (v4l2src->offset + 1)) &&
(v4l2src->offset != 0)) {
-      guint64 lost_frame_count = GST_BUFFER_OFFSET (*buf) - v4l2src->offset -
1;
+    if ((v4l2src->offset != 0) && (GST_BUFFER_OFFSET (*buf) !=
v4l2src->offset)) {
+      guint64 lost_frame_count = GST_BUFFER_OFFSET (*buf) - v4l2src->offset;
       GST_WARNING_OBJECT (v4l2src,
           "lost frames detected: count = %" G_GUINT64_FORMAT " - ts: %"
GST_TIME_FORMAT,
               lost_frame_count, GST_TIME_ARGS (timestamp));
@@ -826,7 +826,7 @@ retry:
       gst_element_post_message (GST_ELEMENT_CAST (v4l2src), qos_msg);

     }
-    v4l2src->offset = GST_BUFFER_OFFSET (*buf);
+    v4l2src->offset = GST_BUFFER_OFFSET_END (*buf);
   }


Not sure if the (micro) optimization to get rid of the '+1' code
is justifying the minor code obfuscation (v4l2src->offset meaning changed
from actual offset to expected offset)?

> 
> @@ +827,3 @@
> +
> +    }
> +    v4l2src->offset = GST_BUFFER_OFFSET (*buf);
> 
> Cool, but we need to reset the offset everything we reset the HW. Otherwise
> we'll send a false positive on every renegotiation.

I have no example for renogotiation at the moment, does the  v4l2 driver
reset the  sequence counter in case of renegotition?

If no: no need to reset v4l2src->offset, lost frame indication will stay...

If yes: I can take a look where to reset v4l2src->offset...

Regards,
Peter

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.