kmssink: Avoid drain on caps changes

Draining systematically on caps changes was a hack. Instead, properly save the render information used to render last_render, and use that information to drain. This fixes performance issues met with video crop meta and per frame caps changes.
2024-07-04 13:45:52 +00:00 · 2019-11-08 17:57:58 -05:00 · 2019-11-08 17:57:58 -05:00 · 8574154cc8
parent a83e0036ea
commit 8574154cc8
2 changed files with 105 additions and 65 deletions
--- a/sys/kms/gstkmssink.c
+++ b/sys/kms/gstkmssink.c
@ -1054,7 +1054,8 @@ config_failed:
 }

 static gboolean
-gst_kms_sink_calculate_display_ratio (GstKMSSink * self, GstVideoInfo * vinfo)
+gst_kms_sink_calculate_display_ratio (GstKMSSink * self, GstVideoInfo * vinfo,
+    gint * scaled_width, gint * scaled_height)
 {
  guint dar_n, dar_d;
  guint video_width, video_height;
@ -1070,8 +1071,8 @@ gst_kms_sink_calculate_display_ratio (GstKMSSink * self, GstVideoInfo * vinfo)
    gst_video_calculate_device_ratio (self->hdisplay, self->vdisplay,
        self->mm_width, self->mm_height, &dpy_par_n, &dpy_par_d);
  } else {
-    GST_VIDEO_SINK_WIDTH (self) = video_width;
-    GST_VIDEO_SINK_HEIGHT (self) = video_height;
+    *scaled_width = video_width;
+    *scaled_height = video_height;
    goto out;
  }

@ -1090,24 +1091,23 @@ gst_kms_sink_calculate_display_ratio (GstKMSSink * self, GstVideoInfo * vinfo)
  /* check hd / dar_d is an integer scale factor, and scale wd with the PAR */
  if (video_height % dar_d == 0) {
    GST_DEBUG_OBJECT (self, "keeping video height");
-    GST_VIDEO_SINK_WIDTH (self) = (guint)
+    *scaled_width = (guint)
        gst_util_uint64_scale_int (video_height, dar_n, dar_d);
-    GST_VIDEO_SINK_HEIGHT (self) = video_height;
+    *scaled_height = video_height;
  } else if (video_width % dar_n == 0) {
    GST_DEBUG_OBJECT (self, "keeping video width");
-    GST_VIDEO_SINK_WIDTH (self) = video_width;
-    GST_VIDEO_SINK_HEIGHT (self) = (guint)
+    *scaled_width = video_width;
+    *scaled_height = (guint)
        gst_util_uint64_scale_int (video_width, dar_d, dar_n);
  } else {
    GST_DEBUG_OBJECT (self, "approximating while keeping video height");
-    GST_VIDEO_SINK_WIDTH (self) = (guint)
+    *scaled_width = (guint)
        gst_util_uint64_scale_int (video_height, dar_n, dar_d);
-    GST_VIDEO_SINK_HEIGHT (self) = video_height;
+    *scaled_height = video_height;
  }

 out:
-  GST_DEBUG_OBJECT (self, "scaling to %dx%d", GST_VIDEO_SINK_WIDTH (self),
-      GST_VIDEO_SINK_HEIGHT (self));
+  GST_DEBUG_OBJECT (self, "scaling to %dx%d", *scaled_width, *scaled_height);

  return TRUE;
 }
@ -1117,44 +1117,34 @@ gst_kms_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
 {
  GstKMSSink *self;
  GstVideoInfo vinfo;
-  GstBufferPool *newpool, *oldpool;

  self = GST_KMS_SINK (bsink);

-  /* We are going to change the internal buffer pool, which means it will no
-   * longer be compatible with the last_buffer size. Drain now, as we won't be
-   * able to do that later on. */
-  gst_kms_sink_drain (self);
-
  if (!gst_video_info_from_caps (&vinfo, caps))
    goto invalid_format;

-  if (!gst_kms_sink_calculate_display_ratio (self, &vinfo))
+  self->last_width = GST_VIDEO_SINK_WIDTH (self);
+  self->last_height = GST_VIDEO_SINK_HEIGHT (self);
+  self->last_vinfo = self->vinfo;
+  self->vinfo = vinfo;
+
+  if (!gst_kms_sink_calculate_display_ratio (self, &vinfo,
+          &GST_VIDEO_SINK_WIDTH (self), &GST_VIDEO_SINK_HEIGHT (self)))
    goto no_disp_ratio;

  if (GST_VIDEO_SINK_WIDTH (self) <= 0 || GST_VIDEO_SINK_HEIGHT (self) <= 0)
    goto invalid_size;

-  /* create a new pool for the new configuration */
-  newpool = gst_kms_sink_create_pool (self, caps, GST_VIDEO_INFO_SIZE (&vinfo),
-      2);
-  if (!newpool)
-    goto no_pool;
-
-  /* we don't activate the internal pool yet as it may not be needed */
-  oldpool = self->pool;
-  self->pool = newpool;
-
-  if (oldpool) {
-    gst_buffer_pool_set_active (oldpool, FALSE);
-    gst_object_unref (oldpool);
+  /* discard dumb buffer pool */
+  if (self->pool) {
+    gst_buffer_pool_set_active (self->pool, FALSE);
+    gst_object_unref (self->pool);
+    self->pool = NULL;
  }

  if (self->modesetting_enabled && !configure_mode_setting (self, &vinfo))
    goto modesetting_failed;

-  self->vinfo = vinfo;
-
  GST_OBJECT_LOCK (self);
  if (self->reconfigure) {
    self->reconfigure = FALSE;
@ -1186,11 +1176,6 @@ no_disp_ratio:
        ("Error calculating the output display ratio of the video."));
    return FALSE;
  }
-no_pool:
-  {
-    /* Already warned in create_pool */
-    return FALSE;
-  }

 modesetting_failed:
  {
@ -1213,6 +1198,8 @@ gst_kms_sink_propose_allocation (GstBaseSink * bsink, GstQuery * query)

  self = GST_KMS_SINK (bsink);

+  GST_DEBUG_OBJECT (self, "propose allocation");
+
  gst_query_parse_allocation (query, &caps, &need_pool);
  if (!caps)
    goto no_caps;
@ -1437,25 +1424,69 @@ wrap_mem:
  return TRUE;
 }

+static gboolean
+ensure_internal_pool (GstKMSSink * self, GstVideoInfo * in_vinfo,
+    GstBuffer * inbuf)
+{
+  GstBufferPool *pool;
+  GstVideoInfo vinfo = *in_vinfo;
+  GstVideoMeta *vmeta;
+  GstCaps *caps;
+
+  if (self->pool)
+    return TRUE;
+
+  /* When cropping, the caps matches the cropped rectangle width/height, but
+   * we can retrieve the padded width/height from the VideoMeta (which is kept
+   * intact when adding crop meta */
+  if ((vmeta = gst_buffer_get_video_meta (inbuf))) {
+    vinfo.width = vmeta->width;
+    vinfo.height = vmeta->height;
+  }
+
+  caps = gst_video_info_to_caps (&vinfo);
+  pool = gst_kms_sink_create_pool (self, caps, gst_buffer_get_size (inbuf), 2);
+  gst_caps_unref (caps);
+
+  if (!pool)
+    return FALSE;
+
+  if (!gst_buffer_pool_set_active (pool, TRUE))
+    goto activate_pool_failed;
+
+  self->pool = pool;
+  return TRUE;
+
+activate_pool_failed:
+  {
+    GST_ELEMENT_ERROR (self, STREAM, FAILED, ("failed to activate buffer pool"),
+        ("failed to activate buffer pool"));
+    gst_object_unref (pool);
+    return FALSE;
+  }
+
+}
+
 static GstBuffer *
-gst_kms_sink_copy_to_dumb_buffer (GstKMSSink * self, GstBuffer * inbuf)
+gst_kms_sink_copy_to_dumb_buffer (GstKMSSink * self, GstVideoInfo * vinfo,
+    GstBuffer * inbuf)
 {
  GstFlowReturn ret;
  GstVideoFrame inframe, outframe;
  gboolean success;
  GstBuffer *buf = NULL;

-  if (!gst_buffer_pool_set_active (self->pool, TRUE))
-    goto activate_pool_failed;
+  if (!ensure_internal_pool (self, vinfo, inbuf))
+    goto bail;

  ret = gst_buffer_pool_acquire_buffer (self->pool, &buf, NULL);
  if (ret != GST_FLOW_OK)
    goto create_buffer_failed;

-  if (!gst_video_frame_map (&inframe, &self->vinfo, inbuf, GST_MAP_READ))
+  if (!gst_video_frame_map (&inframe, vinfo, inbuf, GST_MAP_READ))
    goto error_map_src_buffer;

-  if (!gst_video_frame_map (&outframe, &self->vinfo, buf, GST_MAP_WRITE))
+  if (!gst_video_frame_map (&outframe, vinfo, buf, GST_MAP_WRITE))
    goto error_map_dst_buffer;

  success = gst_video_frame_copy (&outframe, &inframe);
@ -1474,12 +1505,6 @@ bail:
  }

  /* ERRORS */
-activate_pool_failed:
-  {
-    GST_ELEMENT_ERROR (self, STREAM, FAILED, ("failed to activate buffer pool"),
-        ("failed to activate buffer pool"));
-    return NULL;
-  }
 create_buffer_failed:
  {
    GST_ELEMENT_ERROR (self, STREAM, FAILED, ("allocation failed"),
@ -1520,7 +1545,7 @@ gst_kms_sink_get_input_buffer (GstKMSSink * self, GstBuffer * inbuf)
    goto done;

  GST_CAT_INFO_OBJECT (CAT_PERFORMANCE, self, "frame copy");
-  buf = gst_kms_sink_copy_to_dumb_buffer (self, inbuf);
+  buf = gst_kms_sink_copy_to_dumb_buffer (self, &self->vinfo, inbuf);

 done:
  /* Copy all the non-memory related metas, this way CropMeta will be
@ -1538,6 +1563,7 @@ gst_kms_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
  GstBuffer *buffer = NULL;
  guint32 fb_id;
  GstKMSSink *self;
+  GstVideoInfo *vinfo;
  GstVideoCropMeta *crop;
  GstVideoRectangle src = { 0, };
  GstVideoRectangle dst = { 0, };
@ -1548,10 +1574,17 @@ gst_kms_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)

  res = GST_FLOW_ERROR;

-  if (buf)
+  if (buf) {
    buffer = gst_kms_sink_get_input_buffer (self, buf);
-  else if (self->last_buffer)
+    vinfo = &self->vinfo;
+    src.w = GST_VIDEO_SINK_WIDTH (self);
+    src.h = GST_VIDEO_SINK_HEIGHT (self);
+  } else if (self->last_buffer) {
    buffer = gst_buffer_ref (self->last_buffer);
+    vinfo = &self->last_vinfo;
+    src.w = self->last_width;
+    src.h = self->last_height;
+  }

  /* Make sure buf is not used accidentally */
  buf = NULL;
@ -1571,20 +1604,19 @@ gst_kms_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
  }

  if ((crop = gst_buffer_get_video_crop_meta (buffer))) {
-    GstVideoInfo vinfo = self->vinfo;
-    vinfo.width = crop->width;
-    vinfo.height = crop->height;
+    GstVideoInfo cropped_vinfo = *vinfo;

-    if (!gst_kms_sink_calculate_display_ratio (self, &vinfo))
+    cropped_vinfo.width = crop->width;
+    cropped_vinfo.height = crop->height;
+
+    if (!gst_kms_sink_calculate_display_ratio (self, &cropped_vinfo, &src.w,
+            &src.h))
      goto no_disp_ratio;

    src.x = crop->x;
    src.y = crop->y;
  }

-  src.w = GST_VIDEO_SINK_WIDTH (self);
-  src.h = GST_VIDEO_SINK_HEIGHT (self);
-
  dst.w = self->render_rect.w;
  dst.h = self->render_rect.h;

@ -1598,8 +1630,8 @@ retry_set_plane:
    src.w = crop->width;
    src.h = crop->height;
  } else {
-    src.w = GST_VIDEO_INFO_WIDTH (&self->vinfo);
-    src.h = GST_VIDEO_INFO_HEIGHT (&self->vinfo);
+    src.w = GST_VIDEO_INFO_WIDTH (vinfo);
+    src.h = GST_VIDEO_INFO_HEIGHT (vinfo);
  }

  /* handle out of screen case */
@ -1694,11 +1726,16 @@ gst_kms_sink_drain (GstKMSSink * self)
   * In this case, the last_buffer will have a GstParentBufferMeta set. */
  parent_meta = gst_buffer_get_parent_buffer_meta (self->last_buffer);
  if (parent_meta) {
-    GstBuffer *dumb_buf;
-    dumb_buf = gst_kms_sink_copy_to_dumb_buffer (self, parent_meta->buffer);
+    GstBuffer *dumb_buf, *last_buf;
+
+    dumb_buf = gst_kms_sink_copy_to_dumb_buffer (self, &self->last_vinfo,
+        parent_meta->buffer);
+    last_buf = self->last_buffer;
+    self->last_buffer = dumb_buf;
+
    gst_kms_allocator_clear_cache (self->allocator);
-    gst_kms_sink_show_frame (GST_VIDEO_SINK (self), dumb_buf);
-    gst_buffer_unref (dumb_buf);
+    gst_kms_sink_show_frame (GST_VIDEO_SINK (self), NULL);
+    gst_buffer_unref (last_buf);
  }
 }

--- a/sys/kms/gstkmssink.h
+++ b/sys/kms/gstkmssink.h
@ -73,6 +73,9 @@ struct _GstKMSSink {
  GstCaps *allowed_caps;
  GstBufferPool *pool;
  GstAllocator *allocator;
+  GstVideoInfo last_vinfo;
+  guint last_width;
+  guint last_height;
  GstBuffer *last_buffer;
  GstMemory *tmp_kmsmem;