v4l2: Clean up interlace support

Rather than try and guess interlace support as part of checking supported
sizes, look for interlace support specifically in its own function.

As a cleanup, use V4L2_FIELD_ANY when probing sizes, which should result in
the driver doing the right thing.

With my capture setup, this gets me the following sample caps:

For 1080i resolution:
video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)interleaved, framerate=(fraction){ 25/1, 30/1 }

For 720p resolution:
video/x-raw, format=(string)YUY2, width=(int)1280, height=(int)720, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, framerate=(fraction){ 50/1, 60/1 }

For 576i/p resolution (both possible at the point of query):
video/x-raw, format=(string)YUY2, width=(int)720, height=(int)576, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string){ progressive, interleaved }, framerate=(fraction){ 25/1, 50/1 }

This, in turn, makes 576i work correctly; with the old code,
the caps would be interlace-mode=progressive for interlaced video.

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

sys/v4l2/gstv4l2bufferpool.c

@@ -1086,15 +1086,25 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool, GstBuffer ** buffer)
 #endif
 
   /* set top/bottom field first if v4l2_buffer has the information */
-  if (group->buffer.field == V4L2_FIELD_INTERLACED_TB) {
+  switch (group->buffer.field) {
-    GST_BUFFER_FLAG_SET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
+    case V4L2_FIELD_NONE:
-    GST_BUFFER_FLAG_SET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
+      GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
-  } else if (group->buffer.field == V4L2_FIELD_INTERLACED_BT) {
+      GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
-    GST_BUFFER_FLAG_SET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
+      break;
-    GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
+    case V4L2_FIELD_INTERLACED_TB:
-  } else {
+      GST_BUFFER_FLAG_SET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
-    GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
+      GST_BUFFER_FLAG_SET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
-    GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
+      break;
+    case V4L2_FIELD_INTERLACED_BT:
+      GST_BUFFER_FLAG_SET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
+      GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
+      break;
+    default:
+      GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
+      GST_BUFFER_FLAG_UNSET (outbuf, GST_VIDEO_BUFFER_FLAG_TFF);
+      GST_FIXME_OBJECT (pool,
+          "Unhandled enum v4l2_field %d - treating as progressive",
+          group->buffer.field);
   }
 
   if (GST_VIDEO_INFO_FORMAT (&obj->info) == GST_VIDEO_FORMAT_ENCODED) {

sys/v4l2/gstv4l2object.c

@@ -1562,7 +1562,7 @@ unsupported_format:
 
 static gboolean
 gst_v4l2_object_get_nearest_size (GstV4l2Object * v4l2object,
-    guint32 pixelformat, gint * width, gint * height, gboolean * interlaced);
+    guint32 pixelformat, gint * width, gint * height);
 
 static void
 gst_v4l2_object_add_aspect_ratio (GstV4l2Object * v4l2object, GstStructure * s)
@@ -1625,6 +1625,121 @@ gst_v4l2src_value_simplify (GValue * val)
   return FALSE;
 }
 
+static gboolean
+gst_v4l2_object_get_interlace_mode (enum v4l2_field field,
+    GstVideoInterlaceMode * interlace_mode)
+{
+  /* NB: If you add new return values, please fix mode_strings in
+   * gst_v4l2_object_add_interlace_mode */
+  switch (field) {
+    case V4L2_FIELD_ANY:
+      GST_ERROR
+          ("Driver bug detected - check driver with v4l2-compliance from http://git.linuxtv.org/v4l-utils.git\n");
+      /* fallthrough */
+    case V4L2_FIELD_NONE:
+      *interlace_mode = GST_VIDEO_INTERLACE_MODE_PROGRESSIVE;
+      return TRUE;
+    case V4L2_FIELD_INTERLACED:
+    case V4L2_FIELD_INTERLACED_TB:
+    case V4L2_FIELD_INTERLACED_BT:
+      *interlace_mode = GST_VIDEO_INTERLACE_MODE_INTERLEAVED;
+      return TRUE;
+    default:
+      GST_ERROR ("Unknown enum v4l2_field %d", field);
+      return FALSE;
+  }
+}
+
+static int
+gst_v4l2_object_try_fmt (GstV4l2Object * v4l2object,
+    struct v4l2_format *try_fmt)
+{
+  int fd = v4l2object->video_fd;
+  struct v4l2_format fmt;
+  int r;
+
+  memcpy (&fmt, try_fmt, sizeof (fmt));
+  r = v4l2_ioctl (fd, VIDIOC_TRY_FMT, &fmt);
+
+  if (r < 0 && errno == ENOTTY) {
+    /* The driver might not implement TRY_FMT, in which case we will try
+       S_FMT to probe */
+    if (GST_V4L2_IS_ACTIVE (v4l2object))
+      goto error;
+
+    memcpy (&fmt, try_fmt, sizeof (fmt));
+    r = v4l2_ioctl (fd, VIDIOC_S_FMT, &fmt);
+  }
+  memcpy (try_fmt, &fmt, sizeof (fmt));
+  return r;
+
+error:
+  memcpy (try_fmt, &fmt, sizeof (fmt));
+  GST_WARNING_OBJECT (v4l2object->element,
+      "Unable to try format: %s", g_strerror (errno));
+  return r;
+}
+
+
+static void
+gst_v4l2_object_add_interlace_mode (GstV4l2Object * v4l2object,
+    GstStructure * s, guint32 width, guint32 height, guint32 pixelformat)
+{
+  struct v4l2_format fmt;
+  GValue interlace_formats = { 0, };
+  GstVideoInterlaceMode interlace_mode;
+
+  const gchar *mode_strings[] = { "progressive",
+    "interleaved",
+    "mixed"
+  };
+
+  if (!g_str_equal (gst_structure_get_name (s), "video/x-raw"))
+    return;
+
+  if (v4l2object->never_interlaced) {
+    gst_structure_set (s, "interlace-mode", G_TYPE_STRING, "progressive", NULL);
+    return;
+  }
+
+  g_value_init (&interlace_formats, GST_TYPE_LIST);
+
+  /* Try twice - once for NONE, once for INTERLACED. */
+  memset (&fmt, 0, sizeof (fmt));
+  fmt.type = v4l2object->type;
+  fmt.fmt.pix.width = width;
+  fmt.fmt.pix.height = height;
+  fmt.fmt.pix.pixelformat = pixelformat;
+  fmt.fmt.pix.field = V4L2_FIELD_NONE;
+
+  if (gst_v4l2_object_try_fmt (v4l2object, &fmt) == 0 &&
+      gst_v4l2_object_get_interlace_mode (fmt.fmt.pix.field, &interlace_mode)) {
+    GValue interlace_enum = { 0, };
+    g_value_init (&interlace_enum, G_TYPE_STRING);
+    g_value_set_string (&interlace_enum, mode_strings[interlace_mode]);
+    gst_value_list_append_value (&interlace_formats, &interlace_enum);
+  }
+
+  memset (&fmt, 0, sizeof (fmt));
+  fmt.type = v4l2object->type;
+  fmt.fmt.pix.width = width;
+  fmt.fmt.pix.height = height;
+  fmt.fmt.pix.pixelformat = pixelformat;
+  fmt.fmt.pix.field = V4L2_FIELD_INTERLACED;
+
+  if (gst_v4l2_object_try_fmt (v4l2object, &fmt) == 0 &&
+      gst_v4l2_object_get_interlace_mode (fmt.fmt.pix.field, &interlace_mode)) {
+    GValue interlace_enum = { 0, };
+    g_value_init (&interlace_enum, G_TYPE_STRING);
+    g_value_set_string (&interlace_enum, mode_strings[interlace_mode]);
+    gst_value_list_append_value (&interlace_formats, &interlace_enum);
+  }
+
+  gst_v4l2src_value_simplify (&interlace_formats);
+  gst_structure_set_value (s, "interlace-mode", &interlace_formats);
+  return;
+}
+
 /* The frame interval enumeration code first appeared in Linux 2.6.19. */
 static GstStructure *
 gst_v4l2_object_probe_caps_for_format_and_size (GstV4l2Object * v4l2object,
@@ -1636,18 +1751,6 @@ gst_v4l2_object_probe_caps_for_format_and_size (GstV4l2Object * v4l2object,
   guint32 num, denom;
   GstStructure *s;
   GValue rates = { 0, };
-  gboolean interlaced;
-  gint int_width = width;
-  gint int_height = height;
-
-  if (v4l2object->never_interlaced) {
-    interlaced = FALSE;
-  } else {
-    /* Interlaced detection using VIDIOC_TRY/S_FMT */
-    if (!gst_v4l2_object_get_nearest_size (v4l2object, pixelformat,
-            &int_width, &int_height, &interlaced))
-      return NULL;
-  }
 
   memset (&ival, 0, sizeof (struct v4l2_frmivalenum));
   ival.index = 0;
@@ -1807,9 +1910,8 @@ return_data:
   gst_structure_set (s, "width", G_TYPE_INT, (gint) width,
       "height", G_TYPE_INT, (gint) height, NULL);
   gst_v4l2_object_add_aspect_ratio (v4l2object, s);
-  if (g_str_equal (gst_structure_get_name (s), "video/x-raw"))
+  gst_v4l2_object_add_interlace_mode (v4l2object, s, width, height,
-    gst_structure_set (s, "interlace-mode", G_TYPE_STRING,
+      pixelformat);
-        (interlaced ? "mixed" : "progressive"), NULL);
 
   if (G_IS_VALUE (&rates)) {
     gst_v4l2src_value_simplify (&rates);
@@ -2072,19 +2174,18 @@ unknown_type:
 default_frame_sizes:
   {
     gint min_w, max_w, min_h, max_h, fix_num = 0, fix_denom = 0;
-    gboolean interlaced;
 
     /* This code is for Linux < 2.6.19 */
     min_w = min_h = 1;
     max_w = max_h = GST_V4L2_MAX_SIZE;
     if (!gst_v4l2_object_get_nearest_size (v4l2object, pixelformat, &min_w,
-            &min_h, &interlaced)) {
+            &min_h)) {
       GST_WARNING_OBJECT (v4l2object->element,
           "Could not probe minimum capture size for pixelformat %"
           GST_FOURCC_FORMAT, GST_FOURCC_ARGS (pixelformat));
     }
     if (!gst_v4l2_object_get_nearest_size (v4l2object, pixelformat, &max_w,
-            &max_h, &interlaced)) {
+            &max_h)) {
       GST_WARNING_OBJECT (v4l2object->element,
           "Could not probe maximum capture size for pixelformat %"
           GST_FOURCC_FORMAT, GST_FOURCC_ARGS (pixelformat));
@@ -2130,9 +2231,9 @@ default_frame_sizes:
     else
       gst_structure_set (tmp, "height", GST_TYPE_INT_RANGE, min_h, max_h, NULL);
 
-    if (g_str_equal (gst_structure_get_name (tmp), "video/x-raw"))
+    /* We could consider setting interlace mode from min and max. */
-      gst_structure_set (tmp, "interlace-mode", G_TYPE_STRING,
+    gst_v4l2_object_add_interlace_mode (v4l2object, tmp, max_w, max_h,
-          (interlaced ? "mixed" : "progressive"), NULL);
+        pixelformat);
     gst_v4l2_object_add_aspect_ratio (v4l2object, tmp);
 
     gst_v4l2_object_update_and_append (v4l2object, pixelformat, ret, tmp);
@@ -2140,32 +2241,13 @@ default_frame_sizes:
   }
 }
 
-static gboolean
-gst_v4l2_object_get_interlace (int field, gboolean * interlaced)
-{
-  switch (field) {
-    case V4L2_FIELD_ANY:
-    case V4L2_FIELD_NONE:
-      *interlaced = FALSE;
-      return TRUE;
-    case V4L2_FIELD_INTERLACED:
-    case V4L2_FIELD_INTERLACED_TB:
-    case V4L2_FIELD_INTERLACED_BT:
-      *interlaced = TRUE;
-      return TRUE;
-    default:
-      return FALSE;
-  }
-}
-
 static gboolean
 gst_v4l2_object_get_nearest_size (GstV4l2Object * v4l2object,
-    guint32 pixelformat, gint * width, gint * height, gboolean * interlaced)
+    guint32 pixelformat, gint * width, gint * height)
 {
   struct v4l2_format fmt;
-  int fd;
-  int r;
   gboolean ret = FALSE;
+  GstVideoInterlaceMode interlace_mode;
 
   g_return_val_if_fail (width != NULL, FALSE);
   g_return_val_if_fail (height != NULL, FALSE);
@@ -2174,8 +2256,6 @@ gst_v4l2_object_get_nearest_size (GstV4l2Object * v4l2object,
       "getting nearest size to %dx%d with format %" GST_FOURCC_FORMAT,
       *width, *height, GST_FOURCC_ARGS (pixelformat));
 
-  fd = v4l2object->video_fd;
-
   memset (&fmt, 0, sizeof (struct v4l2_format));
 
   /* get size delimiters */
@@ -2184,58 +2264,10 @@ gst_v4l2_object_get_nearest_size (GstV4l2Object * v4l2object,
   fmt.fmt.pix.width = *width;
   fmt.fmt.pix.height = *height;
   fmt.fmt.pix.pixelformat = pixelformat;
-  fmt.fmt.pix.field = V4L2_FIELD_NONE;
+  fmt.fmt.pix.field = V4L2_FIELD_ANY;
 
-  r = v4l2_ioctl (fd, VIDIOC_TRY_FMT, &fmt);
+  if (gst_v4l2_object_try_fmt (v4l2object, &fmt) < 0)
-  if ((r < 0 && errno == EINVAL) ||
+    goto error;
-      !gst_v4l2_object_get_interlace (fmt.fmt.pix.field, interlaced)) {
-    /* try again with interlaced video */
-    memset (&fmt, 0, sizeof (fmt));
-    fmt.type = v4l2object->type;
-    fmt.fmt.pix.width = *width;
-    fmt.fmt.pix.height = *height;
-    fmt.fmt.pix.pixelformat = pixelformat;
-    fmt.fmt.pix.field = V4L2_FIELD_INTERLACED;
-    r = v4l2_ioctl (fd, VIDIOC_TRY_FMT, &fmt);
-  }
-
-  if (r < 0) {
-    /* The driver might not implement TRY_FMT, in which case we will try
-       S_FMT to probe */
-    if (errno != ENOTTY)
-      goto error;
-
-    /* Only try S_FMT if we're not actively capturing yet, which we shouldn't
-       be, because we're still probing */
-    if (GST_V4L2_IS_ACTIVE (v4l2object))
-      goto error;
-
-    GST_LOG_OBJECT (v4l2object->element,
-        "Failed to probe size limit with VIDIOC_TRY_FMT, trying VIDIOC_S_FMT");
-
-    memset (&fmt, 0, sizeof (fmt));
-    fmt.type = v4l2object->type;
-    fmt.fmt.pix.width = *width;
-    fmt.fmt.pix.height = *height;
-    fmt.fmt.pix.pixelformat = pixelformat;
-    fmt.fmt.pix.field = V4L2_FIELD_NONE;
-
-    r = v4l2_ioctl (fd, VIDIOC_S_FMT, &fmt);
-    if ((r < 0 && errno == EINVAL) ||
-        !gst_v4l2_object_get_interlace (fmt.fmt.pix.field, interlaced)) {
-      /* try again with interlaced video */
-      memset (&fmt, 0, sizeof (fmt));
-      fmt.type = v4l2object->type;
-      fmt.fmt.pix.width = *width;
-      fmt.fmt.pix.height = *height;
-      fmt.fmt.pix.pixelformat = pixelformat;
-      fmt.fmt.pix.field = V4L2_FIELD_INTERLACED;
-      r = v4l2_ioctl (fd, VIDIOC_S_FMT, &fmt);
-    }
-
-    if (r < 0)
-      goto error;
-  }
 
   GST_LOG_OBJECT (v4l2object->element,
       "got nearest size %dx%d", fmt.fmt.pix.width, fmt.fmt.pix.height);
@@ -2243,7 +2275,7 @@ gst_v4l2_object_get_nearest_size (GstV4l2Object * v4l2object,
   *width = fmt.fmt.pix.width;
   *height = fmt.fmt.pix.height;
 
-  if (!gst_v4l2_object_get_interlace (fmt.fmt.pix.field, interlaced)) {
+  if (!gst_v4l2_object_get_interlace_mode (fmt.fmt.pix.field, &interlace_mode)) {
     GST_WARNING_OBJECT (v4l2object->element,
         "Unsupported field type for %" GST_FOURCC_FORMAT "@%ux%u: %u",
         GST_FOURCC_ARGS (pixelformat), *width, *height, fmt.fmt.pix.field);

sys/v4l2/gstv4l2src.c

@@ -274,6 +274,9 @@ gst_v4l2src_fixate (GstBaseSrc * basesrc, GstCaps * caps)
 
     if (gst_structure_has_field (structure, "format"))
       gst_structure_fixate_field (structure, "format");
+
+    if (gst_structure_has_field (structure, "interlace-mode"))
+      gst_structure_fixate_field (structure, "interlace-mode");
   }
 
   GST_DEBUG_OBJECT (basesrc, "fixated caps %" GST_PTR_FORMAT, caps);