utils: Unref/release pads in error cases when linking pads

Previously gst_element_link_pads_full() forgot to unreference or release
request pads in several error cases. Also comments were added mentioning
why releasing is not necessary in some places.

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

diff --git a/gst/gstutils.c b/gst/gstutils.c
index 2aacbdc..b98e505 100644 (file)
--- a/gst/gstutils.c
+++ b/gst/gstutils.c
@@ -1503,6 +1503,16 @@ pad_link_maybe_ghosting (GstPad * src, GstPad * sink, GstPadLinkCheck flags)
   return ret;
 }
 
+static void
+release_and_unref_pad (GstElement * element, GstPad * pad, gboolean requestpad)
+{
+  if (pad) {
+    if (requestpad)
+      gst_element_release_request_pad (element, pad);
+    gst_object_unref (pad);
+  }
+}
+
 /**
  * gst_element_link_pads_full:
  * @src: a #GstElement containing the source pad.
@@ -1534,6 +1544,7 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
   GstPad *srcpad, *destpad;
   GstPadTemplate *srctempl, *desttempl;
   GstElementClass *srcclass, *destclass;
+  gboolean srcrequest, destrequest;
 
   /* checks */
   g_return_val_if_fail (GST_IS_ELEMENT (src), FALSE);
@@ -1544,11 +1555,16 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
       srcpadname ? srcpadname : "(any)", GST_ELEMENT_NAME (dest),
       destpadname ? destpadname : "(any)");
 
+  srcrequest = FALSE;
+  destrequest = FALSE;
+
   /* get a src pad */
   if (srcpadname) {
     /* name specified, look it up */
-    if (!(srcpad = gst_element_get_static_pad (src, srcpadname)))
-      srcpad = gst_element_get_request_pad (src, srcpadname);
+    if (!(srcpad = gst_element_get_static_pad (src, srcpadname))) {
+      if ((srcpad = gst_element_get_request_pad (src, srcpadname)))
+        srcrequest = TRUE;
+    }
     if (!srcpad) {
       GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "no pad %s:%s",
           GST_ELEMENT_NAME (src), srcpadname);
@@ -1557,13 +1573,15 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
       if (!(GST_PAD_DIRECTION (srcpad) == GST_PAD_SRC)) {
         GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is no src pad",
             GST_DEBUG_PAD_NAME (srcpad));
-        gst_object_unref (srcpad);
+        release_and_unref_pad (src, srcpad, srcrequest);
         return FALSE;
       }
       if (GST_PAD_PEER (srcpad) != NULL) {
         GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS,
             "pad %s:%s is already linked to %s:%s", GST_DEBUG_PAD_NAME (srcpad),
             GST_DEBUG_PAD_NAME (GST_PAD_PEER (srcpad)));
+        /* already linked request pads look like static pads, so the request pad
+         * was never requested a second time above, so no need to release it */
         gst_object_unref (srcpad);
         return FALSE;
       }
@@ -1582,17 +1600,21 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
   /* get a destination pad */
   if (destpadname) {
     /* name specified, look it up */
-    if (!(destpad = gst_element_get_static_pad (dest, destpadname)))
-      destpad = gst_element_get_request_pad (dest, destpadname);
+    if (!(destpad = gst_element_get_static_pad (dest, destpadname))) {
+      if ((destpad = gst_element_get_request_pad (dest, destpadname)))
+        destrequest = TRUE;
+    }
     if (!destpad) {
       GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "no pad %s:%s",
           GST_ELEMENT_NAME (dest), destpadname);
+      release_and_unref_pad (src, srcpad, srcrequest);
       return FALSE;
     } else {
       if (!(GST_PAD_DIRECTION (destpad) == GST_PAD_SINK)) {
         GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is no sink pad",
             GST_DEBUG_PAD_NAME (destpad));
-        gst_object_unref (destpad);
+        release_and_unref_pad (src, srcpad, srcrequest);
+        release_and_unref_pad (dest, destpad, destrequest);
         return FALSE;
       }
       if (GST_PAD_PEER (destpad) != NULL) {
@@ -1600,6 +1622,9 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
             "pad %s:%s is already linked to %s:%s",
             GST_DEBUG_PAD_NAME (destpad),
             GST_DEBUG_PAD_NAME (GST_PAD_PEER (destpad)));
+        release_and_unref_pad (src, srcpad, srcrequest);
+        /* already linked request pads look like static pads, so the request pad
+         * was never requested a second time above, so no need to release it */
         gst_object_unref (destpad);
         return FALSE;
       }
@@ -1621,9 +1646,13 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
     /* two explicitly specified pads */
     result = pad_link_maybe_ghosting (srcpad, destpad, flags);
 
-    gst_object_unref (srcpad);
-    gst_object_unref (destpad);
-
+    if (result) {
+      gst_object_unref (srcpad);
+      gst_object_unref (destpad);
+    } else {
+      release_and_unref_pad (src, srcpad, srcrequest);
+      release_and_unref_pad (dest, destpad, destrequest);
+    }
     return result;
   }
 
@@ -1637,6 +1666,7 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
           GST_DEBUG_PAD_NAME (srcpad));
       if ((GST_PAD_DIRECTION (srcpad) == GST_PAD_SRC) &&
           (GST_PAD_PEER (srcpad) == NULL)) {
+        gboolean temprequest = FALSE;
         GstPad *temp;
 
         if (destpadname) {
@@ -1644,6 +1674,11 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
           gst_object_ref (temp);
         } else {
           temp = gst_element_get_compatible_pad (dest, srcpad, NULL);
+          if (!GST_IS_GHOST_PAD (temp) &&
+              GST_PAD_TEMPLATE_PRESENCE (GST_PAD_PAD_TEMPLATE (temp)) ==
+              GST_PAD_REQUEST) {
+            temprequest = TRUE;
+          }
         }
 
         if (temp && pad_link_maybe_ghosting (srcpad, temp, flags)) {
@@ -1657,6 +1692,8 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
         }
 
         if (temp) {
+          if (temprequest)
+            gst_element_release_request_pad (dest, temp);
           gst_object_unref (temp);
         }
       }
@@ -1674,13 +1711,16 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
   if (srcpadname) {
     GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "no link possible from %s:%s to %s",
         GST_DEBUG_PAD_NAME (srcpad), GST_ELEMENT_NAME (dest));
+    /* no need to release any request pad as both src- and destpadname must be
+     * set to end up here, but this case has already been taken care of above */
     if (destpad)
       gst_object_unref (destpad);
     destpad = NULL;
   }
-  if (srcpad)
-    gst_object_unref (srcpad);
-  srcpad = NULL;
+  if (srcpad) {
+    release_and_unref_pad (src, srcpad, srcrequest);
+    srcpad = NULL;
+  }
 
   if (destpad) {
     /* loop through the existing pads in the destination */
@@ -1690,6 +1730,15 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
       if ((GST_PAD_DIRECTION (destpad) == GST_PAD_SINK) &&
           (GST_PAD_PEER (destpad) == NULL)) {
         GstPad *temp = gst_element_get_compatible_pad (src, destpad, NULL);
+        gboolean temprequest = FALSE;
+
+        if (temp) {
+          if (!GST_IS_GHOST_PAD (temp) &&
+              GST_PAD_TEMPLATE_PRESENCE (GST_PAD_PAD_TEMPLATE (temp)) ==
+              GST_PAD_REQUEST) {
+            temprequest = TRUE;
+          }
+        }
 
         if (temp && pad_link_maybe_ghosting (temp, destpad, flags)) {
           GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "linked pad %s:%s to pad %s:%s",
@@ -1698,9 +1747,8 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
           gst_object_unref (destpad);
           return TRUE;
         }
-        if (temp) {
-          gst_object_unref (temp);
-        }
+
+        release_and_unref_pad (src, temp, temprequest);
       }
       if (destpads) {
         destpads = g_list_next (destpads);
@@ -1716,11 +1764,15 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
   if (destpadname) {
     GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "no link possible from %s to %s:%s",
         GST_ELEMENT_NAME (src), GST_DEBUG_PAD_NAME (destpad));
-    gst_object_unref (destpad);
+    release_and_unref_pad (dest, destpad, destrequest);
     return FALSE;
   } else {
-    if (destpad)
+    /* no need to release any request pad as the case of unset destpatname and
+     * destpad being a requst pad has already been taken care of when looking
+     * though the destination pads above */
+    if (destpad) {
       gst_object_unref (destpad);
+    }
     destpad = NULL;
   }
 
@@ -1763,10 +1815,14 @@ gst_element_link_pads_full (GstElement * src, const gchar * srcpadname,
                 return TRUE;
               }
               /* it failed, so we release the request pads */
-              if (srcpad)
+              if (srcpad) {
                 gst_element_release_request_pad (src, srcpad);
-              if (destpad)
+                gst_object_unref (srcpad);
+              }
+              if (destpad) {
                 gst_element_release_request_pad (dest, destpad);
+                gst_object_unref (destpad);
+              }
             }
             gst_caps_unref (srccaps);
             gst_caps_unref (destcaps);