From d5067b42dedeffab121b9a0359966404b09a7378 Mon Sep 17 00:00:00 2001
From: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date: Mon, 20 Nov 2017 17:54:54 +0100
Subject: [PATCH] h264parse: put downstream caps first if possible on sink caps

Try prioritizing downstream's caps over upstream's if possible so the
parser can configured in "passthrough" if possible and save it from
doing useless conversions.

https://bugzilla.gnome.org/show_bug.cgi?id=790628
---
 gst/videoparsers/gsth264parse.c  | 35 ++++++++++++---------
 tests/check/elements/h264parse.c | 54 ++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 14 deletions(-)

diff --git a/gst/videoparsers/gsth264parse.c b/gst/videoparsers/gsth264parse.c
index 10543eafcd..eb5197e75b 100644
--- a/gst/videoparsers/gsth264parse.c
+++ b/gst/videoparsers/gsth264parse.c
@@ -2752,7 +2752,7 @@ refuse_caps:
 }
 
 static void
-remove_fields (GstCaps * caps)
+remove_fields (GstCaps * caps, gboolean all)
 {
   guint i, n;
 
@@ -2760,8 +2760,10 @@ remove_fields (GstCaps * caps)
   for (i = 0; i < n; i++) {
     GstStructure *s = gst_caps_get_structure (caps, i);
 
-    gst_structure_remove_field (s, "alignment");
-    gst_structure_remove_field (s, "stream-format");
+    if (all) {
+      gst_structure_remove_field (s, "alignment");
+      gst_structure_remove_field (s, "stream-format");
+    }
     gst_structure_remove_field (s, "parsed");
   }
 }
@@ -2770,28 +2772,24 @@ static GstCaps *
 gst_h264_parse_get_caps (GstBaseParse * parse, GstCaps * filter)
 {
   GstCaps *peercaps, *templ;
-  GstCaps *res;
+  GstCaps *res, *tmp, *pcopy;
 
   templ = gst_pad_get_pad_template_caps (GST_BASE_PARSE_SINK_PAD (parse));
   if (filter) {
     GstCaps *fcopy = gst_caps_copy (filter);
     /* Remove the fields we convert */
-    remove_fields (fcopy);
+    remove_fields (fcopy, TRUE);
     peercaps = gst_pad_peer_query_caps (GST_BASE_PARSE_SRC_PAD (parse), fcopy);
     gst_caps_unref (fcopy);
   } else
     peercaps = gst_pad_peer_query_caps (GST_BASE_PARSE_SRC_PAD (parse), NULL);
 
-  if (peercaps) {
-    peercaps = gst_caps_make_writable (peercaps);
-    remove_fields (peercaps);
+  pcopy = gst_caps_copy (peercaps);
+  remove_fields (pcopy, TRUE);
 
-    res = gst_caps_intersect_full (peercaps, templ, GST_CAPS_INTERSECT_FIRST);
-    gst_caps_unref (peercaps);
-    gst_caps_unref (templ);
-  } else {
-    res = templ;
-  }
+  res = gst_caps_intersect_full (pcopy, templ, GST_CAPS_INTERSECT_FIRST);
+  gst_caps_unref (pcopy);
+  gst_caps_unref (templ);
 
   if (filter) {
     GstCaps *tmp = gst_caps_intersect_full (res, filter,
@@ -2800,6 +2798,15 @@ gst_h264_parse_get_caps (GstBaseParse * parse, GstCaps * filter)
     res = tmp;
   }
 
+  /* Try if we can put the downstream caps first */
+  remove_fields (peercaps, FALSE);
+  tmp = gst_caps_intersect_full (peercaps, res, GST_CAPS_INTERSECT_FIRST);
+  if (!gst_caps_is_empty (tmp))
+    res = gst_caps_merge (tmp, res);
+  else
+    gst_caps_unref (tmp);
+
+  gst_caps_unref (peercaps);
   return res;
 }
 
diff --git a/tests/check/elements/h264parse.c b/tests/check/elements/h264parse.c
index 65fbc90e1d..5a6d3e4727 100644
--- a/tests/check/elements/h264parse.c
+++ b/tests/check/elements/h264parse.c
@@ -344,6 +344,59 @@ GST_START_TEST (test_parse_detect_stream)
 
 GST_END_TEST;
 
+static GstStaticPadTemplate srctemplate_avc_au_and_bs_au =
+    GST_STATIC_PAD_TEMPLATE ("src",
+    GST_PAD_SRC,
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS (SRC_CAPS_TMPL
+        ", stream-format = (string) avc, alignment = (string) au; "
+        SRC_CAPS_TMPL
+        ", stream-format = (string) byte-stream, alignment = (string) au")
+    );
+
+GST_START_TEST (test_sink_caps_reordering)
+{
+  /* Upstream can handle avc and byte-stream format (in that preference order)
+   * and downstream requires byte-stream.
+   * Parser reorder upstream's caps to prefer the format requested downstream
+   * and so avoid doing useless conversions. */
+  GstElement *parser;
+  GstPad *sink, *src;
+  GstCaps *src_caps, *sink_caps;
+  GstStructure *s;
+
+  parser = gst_check_setup_element ("h264parse");
+  fail_unless (parser);
+
+  src = gst_check_setup_src_pad (parser, &srctemplate_avc_au_and_bs_au);
+  sink = gst_check_setup_sink_pad (parser, &sinktemplate_bs_au);
+
+  src_caps = gst_pad_get_pad_template_caps (src);
+  sink_caps = gst_pad_peer_query_caps (src, src_caps);
+
+  /* Sink pad has both format on its sink caps but prefer to use byte-stream */
+  g_assert_cmpuint (gst_caps_get_size (sink_caps), ==, 2);
+
+  s = gst_caps_get_structure (sink_caps, 0);
+  g_assert_cmpstr (gst_structure_get_name (s), ==, "video/x-h264");
+  g_assert_cmpstr (gst_structure_get_string (s, "alignment"), ==, "au");
+  g_assert_cmpstr (gst_structure_get_string (s, "stream-format"), ==,
+      "byte-stream");
+
+  s = gst_caps_get_structure (sink_caps, 1);
+  g_assert_cmpstr (gst_structure_get_name (s), ==, "video/x-h264");
+  g_assert_cmpstr (gst_structure_get_string (s, "alignment"), ==, "au");
+  g_assert_cmpstr (gst_structure_get_string (s, "stream-format"), ==, "avc");
+
+  gst_caps_unref (src_caps);
+  gst_caps_unref (sink_caps);
+  gst_object_unref (src);
+  gst_object_unref (sink);
+  gst_object_unref (parser);
+}
+
+GST_END_TEST;
+
 
 static Suite *
 h264parse_suite (void)
@@ -358,6 +411,7 @@ h264parse_suite (void)
   tcase_add_test (tc_chain, test_parse_split);
   tcase_add_test (tc_chain, test_parse_skip_garbage);
   tcase_add_test (tc_chain, test_parse_detect_stream);
+  tcase_add_test (tc_chain, test_sink_caps_reordering);
 
   return s;
 }