From 85f297d648a71e89630cdf8de149af0341d2bd31 Mon Sep 17 00:00:00 2001
From: Stian Selnes <stian@pexip.com>
Date: Tue, 26 Jan 2016 13:54:46 +0100
Subject: [PATCH] videoencoder: Fix leak when pre_push does not return OK

https://bugzilla.gnome.org/show_bug.cgi?id=761951
---
 gst-libs/gst/video/gstvideoencoder.c |  3 +-
 tests/check/libs/videoencoder.c      | 52 +++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/gst-libs/gst/video/gstvideoencoder.c b/gst-libs/gst/video/gstvideoencoder.c
index c45bc7d382..a13d4ed2eb 100644
--- a/gst-libs/gst/video/gstvideoencoder.c
+++ b/gst-libs/gst/video/gstvideoencoder.c
@@ -2183,7 +2183,8 @@ gst_video_encoder_finish_frame (GstVideoEncoder * encoder,
 
   /* Get an additional ref to the buffer, which is going to be pushed
    * downstream, the original ref is owned by the frame */
-  buffer = gst_buffer_ref (frame->output_buffer);
+  if (ret == GST_FLOW_OK)
+    buffer = gst_buffer_ref (frame->output_buffer);
 
   /* Release frame so the buffer is writable when we push it downstream
    * if possible, i.e. if the subclass does not hold additional references
diff --git a/tests/check/libs/videoencoder.c b/tests/check/libs/videoencoder.c
index af7e06c61b..1844ab69be 100644
--- a/tests/check/libs/videoencoder.c
+++ b/tests/check/libs/videoencoder.c
@@ -24,6 +24,7 @@
 #endif
 #include <gst/gst.h>
 #include <gst/check/gstcheck.h>
+#include <gst/check/gstharness.h>
 #include <gst/video/video.h>
 #include <gst/app/app.h>
 
@@ -45,6 +46,8 @@ typedef struct _GstVideoEncoderTesterClass GstVideoEncoderTesterClass;
 struct _GstVideoEncoderTester
 {
   GstVideoEncoder parent;
+
+  GstFlowReturn pre_push_result;
 };
 
 struct _GstVideoEncoderTesterClass
@@ -102,6 +105,15 @@ gst_video_encoder_tester_handle_frame (GstVideoEncoder * dec,
   return gst_video_encoder_finish_frame (dec, frame);
 }
 
+static GstFlowReturn
+gst_video_encoder_tester_pre_push (GstVideoEncoder * enc,
+    GstVideoCodecFrame * frame)
+{
+  GstVideoEncoderTester *tester = (GstVideoEncoderTester *) enc;
+
+  return tester->pre_push_result;
+}
+
 static void
 gst_video_encoder_tester_class_init (GstVideoEncoderTesterClass * klass)
 {
@@ -127,12 +139,14 @@ gst_video_encoder_tester_class_init (GstVideoEncoderTesterClass * klass)
   videoencoder_class->start = gst_video_encoder_tester_start;
   videoencoder_class->stop = gst_video_encoder_tester_stop;
   videoencoder_class->handle_frame = gst_video_encoder_tester_handle_frame;
+  videoencoder_class->pre_push = gst_video_encoder_tester_pre_push;
   videoencoder_class->set_format = gst_video_encoder_tester_set_format;
 }
 
 static void
 gst_video_encoder_tester_init (GstVideoEncoderTester * tester)
 {
+  tester->pre_push_result = GST_FLOW_OK;
 }
 
 static gboolean
@@ -199,6 +213,15 @@ create_test_buffer (guint64 num)
   return buffer;
 }
 
+static GstCaps *
+create_test_caps (void)
+{
+  return gst_caps_new_simple ("video/x-raw", "width", G_TYPE_INT,
+      TEST_VIDEO_WIDTH, "height", G_TYPE_INT, TEST_VIDEO_HEIGHT, "framerate",
+      GST_TYPE_FRACTION, TEST_VIDEO_FPS_N, TEST_VIDEO_FPS_D,
+      "format", G_TYPE_STRING, "GRAY8", NULL);
+}
+
 static void
 send_startup_events (void)
 {
@@ -208,11 +231,7 @@ send_startup_events (void)
           gst_event_new_stream_start ("randomvalue")));
 
   /* push caps */
-  caps =
-      gst_caps_new_simple ("video/x-raw", "width", G_TYPE_INT,
-      TEST_VIDEO_WIDTH, "height", G_TYPE_INT, TEST_VIDEO_HEIGHT, "framerate",
-      GST_TYPE_FRACTION, TEST_VIDEO_FPS_N, TEST_VIDEO_FPS_D,
-      "format", G_TYPE_STRING, "GRAY8", NULL);
+  caps = create_test_caps ();
   fail_unless (gst_pad_push_event (mysrcpad, gst_event_new_caps (caps)));
   gst_caps_unref (caps);
 }
@@ -495,6 +514,28 @@ GST_START_TEST (videoencoder_flush_events)
 
 GST_END_TEST;
 
+/* When pre_push fails the correct GstFlowReturn should be returned and there
+ * should be no leaks */
+GST_START_TEST (videoencoder_pre_push_fails)
+{
+  GstVideoEncoderTester *tester;
+  GstHarness *h;
+
+  tester = g_object_new (GST_VIDEO_ENCODER_TESTER_TYPE, NULL);
+  tester->pre_push_result = GST_FLOW_ERROR;
+
+  h = gst_harness_new_with_element (GST_ELEMENT (tester), "sink", "src");
+  gst_harness_set_src_caps (h, create_test_caps ());
+
+  fail_unless_equals_int (gst_harness_push (h, create_test_buffer (0)),
+      GST_FLOW_ERROR);
+
+  gst_harness_teardown (h);
+  gst_object_unref (tester);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_videoencoder_suite (void)
 {
@@ -507,6 +548,7 @@ gst_videoencoder_suite (void)
   tcase_add_test (tc, videoencoder_tags_before_eos);
   tcase_add_test (tc, videoencoder_events_before_eos);
   tcase_add_test (tc, videoencoder_flush_events);
+  tcase_add_test (tc, videoencoder_pre_push_fails);
 
   return s;
 }