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.

2 files changed, 105 insertions, 65 deletions

diff --git a/sys/kms/gstkmssink.c b/sys/kms/gstkmssink.c
index 23718e9c5..4f6ed26eb 100644
--- 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;
+
+    cropped_vinfo.width = crop->width;
+    cropped_vinfo.height = crop->height;
 
-    if (!gst_kms_sink_calculate_display_ratio (self, &vinfo))
+    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);
   }
 }
 
diff --git a/sys/kms/gstkmssink.h b/sys/kms/gstkmssink.h
index 65407dab4..4b1ebeac3 100644
--- 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;