pad: improve docs of get/pull_range

Improve the docs of the get/pull_range functions, define the lifetime of the
buffer in case of errors and short reads.
Make sure the code does what the docs say.

gst/gstpad.c

@@ -3818,6 +3818,7 @@ gst_pad_get_range_unchecked (GstPad * pad, guint64 offset, guint size,
   GstFlowReturn ret;
   GstPadGetRangeFunction getrangefunc;
   GstObject *parent;
+  GstBuffer *res_buf;
 
   GST_PAD_STREAM_LOCK (pad);
 
@@ -3831,11 +3832,13 @@ gst_pad_get_range_unchecked (GstPad * pad, guint64 offset, guint size,
   if (G_UNLIKELY ((ret = check_sticky (pad))) != GST_FLOW_OK)
     goto events_error;
 
+  res_buf = *buffer;
+
   /* when one of the probes returns DROPPED, probe_stopped will be called
-   * and we skip calling the getrange function, *buffer should then contain a
+   * and we skip calling the getrange function, res_buf should then contain a
    * valid result buffer */
   PROBE_PULL (pad, GST_PAD_PROBE_TYPE_PULL | GST_PAD_PROBE_TYPE_BLOCK,
-      *buffer, offset, size, probe_stopped);
+      res_buf, offset, size, probe_stopped);
 
   ACQUIRE_PARENT (pad, parent, no_parent);
   GST_OBJECT_UNLOCK (pad);
@@ -3848,7 +3851,7 @@ gst_pad_get_range_unchecked (GstPad * pad, guint64 offset, guint size,
       G_GUINT64_FORMAT ", size %u",
       GST_DEBUG_FUNCPTR_NAME (getrangefunc), offset, size);
 
-  ret = getrangefunc (pad, parent, offset, size, buffer);
+  ret = getrangefunc (pad, parent, offset, size, &res_buf);
 
   RELEASE_PARENT (parent);
 
@@ -3859,11 +3862,13 @@ gst_pad_get_range_unchecked (GstPad * pad, guint64 offset, guint size,
   GST_OBJECT_LOCK (pad);
 probed_data:
   PROBE_PULL (pad, GST_PAD_PROBE_TYPE_PULL | GST_PAD_PROBE_TYPE_BUFFER,
-      *buffer, offset, size, probe_stopped_unref);
+      res_buf, offset, size, probe_stopped_unref);
   GST_OBJECT_UNLOCK (pad);
 
   GST_PAD_STREAM_UNLOCK (pad);
 
+  *buffer = res_buf;
+
   return ret;
 
   /* ERRORS */
@@ -3910,7 +3915,7 @@ probe_stopped:
     GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad,
         "probe returned %s", gst_flow_get_name (ret));
     if (ret == GST_FLOW_CUSTOM_SUCCESS) {
-      if (*buffer) {
+      if (res_buf) {
         /* the probe filled the buffer and asks us to not call the getrange
          * anymore, we continue with the post probes then. */
         GST_DEBUG_OBJECT (pad, "handled buffer");
@@ -3936,14 +3941,13 @@ probe_stopped_unref:
       ret = GST_FLOW_EOS;
     GST_OBJECT_UNLOCK (pad);
     GST_PAD_STREAM_UNLOCK (pad);
-    gst_buffer_unref (*buffer);
+    if (*buffer == NULL)
-    *buffer = NULL;
+      gst_buffer_unref (res_buf);
     return ret;
   }
 get_range_failed:
   {
     GST_PAD_STREAM_UNLOCK (pad);
-    *buffer = NULL;
     GST_CAT_LEVEL_LOG (GST_CAT_SCHEDULING,
         (ret >= GST_FLOW_EOS) ? GST_LEVEL_INFO : GST_LEVEL_WARNING,
         pad, "getrange failed, flow: %s", gst_flow_get_name (ret));
@@ -3967,8 +3971,22 @@ get_range_failed:
  * installed (see gst_pad_set_getrange_function()) this function returns
  * #GST_FLOW_NOT_SUPPORTED.
  *
- * @buffer must point to a variable holding NULL or a variable that
+ * If @buffer points to a variable holding NULL, a valid new #GstBuffer will be
- * points to a #GstBuffer that will be filled with the result data.
+ * placed in @buffer when this function returns #GST_FLOW_OK. The new buffer
+ * must be freed with gst_buffer_unref() after usage.
+ *
+ * When @buffer points to a variable that points to a valid #GstBuffer, the
+ * buffer will be filled with the result data when this function returns
+ * #GST_FLOW_OK. If the provided buffer is larger than @size, only
+ * @size bytes will be filled in the result buffer and its size will be updated
+ * accordingly.
+ *
+ * Note that less than @size bytes can be returned in @buffer when, for example,
+ * an EOS condition is near or when @buffer is not large enough to hold @size
+ * bytes. The caller should check the result buffer size to get the result size.
+ *
+ * When this function returns any other result value than #GST_FLOW_OK, @buffer
+ * will be unchanged.
  *
  * This is a lowlevel function. Usualy gst_pad_pull_range() is used.
  *
@@ -3997,7 +4015,7 @@ gst_pad_get_range (GstPad * pad, guint64 offset, guint size,
  * @buffer: (out callee-allocates): a pointer to hold the #GstBuffer, returns
  *     GST_FLOW_ERROR if %NULL.
  *
- * Pulls a @buffer from the peer pad.
+ * Pulls a @buffer from the peer pad or fills up a provided buffer.
  *
  * This function will first trigger the pad block signal if it was
  * installed.
@@ -4007,14 +4025,23 @@ gst_pad_get_range (GstPad * pad, guint64 offset, guint size,
  * See gst_pad_get_range() for a list of return values and for the
  * semantics of the arguments of this function.
  *
- * @buffer must point to a variable holding NULL or a variable that
+ * If @buffer points to a variable holding NULL, a valid new #GstBuffer will be
- * points to a #GstBuffer that will be filled with the result data.
+ * placed in @buffer when this function returns #GST_FLOW_OK. The new buffer
+ * must be freed with gst_buffer_unref() after usage. When this function
+ * returns any other result value, @buffer will still point to NULL.
+ *
+ * When @buffer points to a variable that points to a valid #GstBuffer, the
+ * buffer will be filled with the result data when this function returns
+ * #GST_FLOW_OK. When this function returns any other result value,
+ * @buffer will be unchanged. If the provided buffer is larger than @size, only
+ * @size bytes will be filled in the result buffer and its size will be updated
+ * accordingly.
+ *
+ * Note that less than @size bytes can be returned in @buffer when, for example,
+ * an EOS condition is near or when @buffer is not large enough to hold @size
+ * bytes. The caller should check the result buffer size to get the result size.
  *
  * Returns: a #GstFlowReturn from the peer pad.
- * When this function returns #GST_FLOW_OK, @buffer will contain a valid
- * #GstBuffer that should be freed with gst_buffer_unref() after usage.
- * @buffer may not be used or freed when any other return value than
- * #GST_FLOW_OK is returned.
  *
  * MT safe.
  */
@@ -4024,6 +4051,7 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size,
 {
   GstPad *peer;
   GstFlowReturn ret;
+  GstBuffer *res_buf;
 
   g_return_val_if_fail (GST_IS_PAD (pad), GST_FLOW_ERROR);
   g_return_val_if_fail (GST_PAD_IS_SINK (pad), GST_FLOW_ERROR);
@@ -4038,11 +4066,13 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size,
   if (G_UNLIKELY (GST_PAD_MODE (pad) != GST_PAD_MODE_PULL))
     goto wrong_mode;
 
+  res_buf = *buffer;
+
   /* when one of the probes returns DROPPED, probe_stopped will be
    * called and we skip calling the peer getrange function. *buffer should then
    * contain a valid buffer */
   PROBE_PULL (pad, GST_PAD_PROBE_TYPE_PULL | GST_PAD_PROBE_TYPE_BLOCK,
-      *buffer, offset, size, probe_stopped);
+      res_buf, offset, size, probe_stopped);
 
   if (G_UNLIKELY ((peer = GST_PAD_PEER (pad)) == NULL))
     goto not_linked;
@@ -4051,7 +4081,7 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size,
   pad->priv->using++;
   GST_OBJECT_UNLOCK (pad);
 
-  ret = gst_pad_get_range_unchecked (peer, offset, size, buffer);
+  ret = gst_pad_get_range_unchecked (peer, offset, size, &res_buf);
 
   gst_object_unref (peer);
 
@@ -4068,10 +4098,12 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size,
 
 probed_data:
   PROBE_PULL (pad, GST_PAD_PROBE_TYPE_PULL | GST_PAD_PROBE_TYPE_BUFFER,
-      *buffer, offset, size, probe_stopped_unref);
+      res_buf, offset, size, probe_stopped_unref);
 
   GST_OBJECT_UNLOCK (pad);
 
+  *buffer = res_buf;
+
   return ret;
 
   /* ERROR recovery here */
@@ -4094,7 +4126,7 @@ probe_stopped:
     GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "pre probe returned %s",
         gst_flow_get_name (ret));
     if (ret == GST_FLOW_CUSTOM_SUCCESS) {
-      if (*buffer) {
+      if (res_buf) {
         /* the probe filled the buffer and asks us to not forward to the peer
          * anymore, we continue with the post probes then */
         GST_DEBUG_OBJECT (pad, "handled buffer");
@@ -4118,7 +4150,6 @@ not_linked:
   }
 pull_range_failed:
   {
-    *buffer = NULL;
     GST_OBJECT_UNLOCK (pad);
     GST_CAT_LEVEL_LOG (GST_CAT_SCHEDULING,
         (ret >= GST_FLOW_EOS) ? GST_LEVEL_INFO : GST_LEVEL_WARNING,
@@ -4131,12 +4162,10 @@ probe_stopped_unref:
         "post probe returned %s", gst_flow_get_name (ret));
     GST_OBJECT_UNLOCK (pad);
     /* if we drop here, it signals EOS */
-    if (ret == GST_FLOW_CUSTOM_SUCCESS) {
+    if (ret == GST_FLOW_CUSTOM_SUCCESS)
       ret = GST_FLOW_EOS;
-      gst_buffer_unref (*buffer);
+    if (*buffer == NULL)
-    } else if (ret == GST_FLOW_OK)
+      gst_buffer_unref (res_buf);
-      gst_buffer_unref (*buffer);
-    *buffer = NULL;
     return ret;
   }
 }

libs/gst/base/gstbasesrc.c

@@ -1379,6 +1379,7 @@ gst_base_src_default_create (GstBaseSrc * src, guint64 offset,
 {
   GstBaseSrcClass *bclass;
   GstFlowReturn ret;
+  GstBuffer *res_buf;
 
   bclass = GST_BASE_SRC_GET_CLASS (src);
 
@@ -1390,18 +1391,22 @@ gst_base_src_default_create (GstBaseSrc * src, guint64 offset,
   if (*buffer == NULL) {
     /* downstream did not provide us with a buffer to fill, allocate one
      * ourselves */
-    ret = bclass->alloc (src, offset, size, buffer);
+    ret = bclass->alloc (src, offset, size, &res_buf);
     if (G_UNLIKELY (ret != GST_FLOW_OK))
       goto alloc_failed;
+  } else {
+    res_buf = *buffer;
   }
 
   if (G_LIKELY (size > 0)) {
     /* only call fill when there is a size */
-    ret = bclass->fill (src, offset, size, *buffer);
+    ret = bclass->fill (src, offset, size, res_buf);
     if (G_UNLIKELY (ret != GST_FLOW_OK))
       goto not_ok;
   }
 
+  *buffer = res_buf;
+
   return GST_FLOW_OK;
 
   /* ERRORS */
@@ -1419,7 +1424,8 @@ not_ok:
   {
     GST_DEBUG_OBJECT (src, "fill returned %d (%s)", ret,
         gst_flow_get_name (ret));
-    gst_buffer_unref (*buffer);
+    if (*buffer == NULL)
+      gst_buffer_unref (res_buf);
     return ret;
   }
 }
@@ -2242,6 +2248,7 @@ gst_base_src_get_range (GstBaseSrc * src, guint64 offset, guint length,
   GstFlowReturn ret;
   GstBaseSrcClass *bclass;
   GstClockReturn status;
+  GstBuffer *res_buf;
 
   bclass = GST_BASE_SRC_GET_CLASS (src);
 
@@ -2287,15 +2294,17 @@ again:
       "calling create offset %" G_GUINT64_FORMAT " length %u, time %"
       G_GINT64_FORMAT, offset, length, src->segment.time);
 
-  ret = bclass->create (src, offset, length, buf);
+  res_buf = *buf;
+
+  ret = bclass->create (src, offset, length, &res_buf);
 
   /* The create function could be unlocked because we have a pending EOS. It's
    * possible that we have a valid buffer from create that we need to
    * discard when the create function returned _OK. */
   if (G_UNLIKELY (g_atomic_int_get (&src->priv->pending_eos))) {
     if (ret == GST_FLOW_OK) {
-      gst_buffer_unref (*buf);
+      if (*buf == NULL)
-      *buf = NULL;
+        gst_buffer_unref (res_buf);
     }
     goto eos;
   }
@@ -2305,14 +2314,14 @@ again:
 
   /* no timestamp set and we are at offset 0, we can timestamp with 0 */
   if (offset == 0 && src->segment.time == 0
-      && GST_BUFFER_TIMESTAMP (*buf) == -1 && !src->is_live) {
+      && GST_BUFFER_TIMESTAMP (res_buf) == -1 && !src->is_live) {
     GST_DEBUG_OBJECT (src, "setting first timestamp to 0");
-    *buf = gst_buffer_make_writable (*buf);
+    res_buf = gst_buffer_make_writable (res_buf);
-    GST_BUFFER_TIMESTAMP (*buf) = 0;
+    GST_BUFFER_TIMESTAMP (res_buf) = 0;
   }
 
   /* now sync before pushing the buffer */
-  status = gst_base_src_do_sync (src, *buf);
+  status = gst_base_src_do_sync (src, res_buf);
 
   /* waiting for the clock could have made us flushing */
   if (G_UNLIKELY (src->priv->flushing))
@@ -2331,8 +2340,9 @@ again:
       /* this case is triggered when we were waiting for the clock and
        * it got unlocked because we did a state change. In any case, get rid of
        * the buffer. */
-      gst_buffer_unref (*buf);
+      if (*buf == NULL)
-      *buf = NULL;
+        gst_buffer_unref (res_buf);
+
       if (!src->live_running) {
         /* We return WRONG_STATE when we are not running to stop the dataflow also
          * get rid of the produced buffer. */
@@ -2352,11 +2362,14 @@ again:
       GST_ELEMENT_ERROR (src, CORE, CLOCK,
           (_("Internal clock error.")),
           ("clock returned unexpected return value %d", status));
-      gst_buffer_unref (*buf);
+      if (*buf == NULL)
-      *buf = NULL;
+        gst_buffer_unref (res_buf);
       ret = GST_FLOW_ERROR;
       break;
   }
+  if (G_LIKELY (ret == GST_FLOW_OK))
+    *buf = res_buf;
+
   return ret;
 
   /* ERROR */
@@ -2396,8 +2409,8 @@ reached_num_buffers:
 flushing:
   {
     GST_DEBUG_OBJECT (src, "we are flushing");
-    gst_buffer_unref (*buf);
+    if (*buf == NULL)
-    *buf = NULL;
+      gst_buffer_unref (res_buf);
     return GST_FLOW_FLUSHING;
   }
 eos:

plugins/elements/gstqueue2.c

@@ -1161,7 +1161,11 @@ gst_queue2_create_read (GstQueue2 * queue, guint64 offset, guint length,
   GstFlowReturn ret = GST_FLOW_OK;
 
   /* allocate the output buffer of the requested size */
+  if (*buffer == NULL)
     buf = gst_buffer_new_allocate (NULL, length, NULL);
+  else
+    buf = *buffer;
+
   gst_buffer_map (buf, &info, GST_MAP_WRITE);
   data = info.data;
 
@@ -1289,6 +1293,7 @@ hit_eos:
   {
     GST_DEBUG_OBJECT (queue, "EOS hit and we don't have any requested data");
     gst_buffer_unmap (buf, &info);
+    if (*buffer == NULL)
       gst_buffer_unref (buf);
     return GST_FLOW_EOS;
   }
@@ -1296,6 +1301,7 @@ out_flushing:
   {
     GST_DEBUG_OBJECT (queue, "we are flushing");
     gst_buffer_unmap (buf, &info);
+    if (*buffer == NULL)
       gst_buffer_unref (buf);
     return GST_FLOW_FLUSHING;
   }
@@ -1303,6 +1309,7 @@ read_error:
   {
     GST_DEBUG_OBJECT (queue, "we have a read error");
     gst_buffer_unmap (buf, &info);
+    if (*buffer == NULL)
       gst_buffer_unref (buf);
     return ret;
   }
@@ -1322,7 +1329,7 @@ gst_queue2_read_item_from_file (GstQueue2 * queue)
     queue->starting_segment = NULL;
   } else {
     GstFlowReturn ret;
-    GstBuffer *buffer;
+    GstBuffer *buffer = NULL;
     guint64 reading_pos;
 
     reading_pos = queue->current->reading_pos;