souphttpclientsink: Fix lifetime of stream headers and queued buffers
authorNirbheek Chauhan <nirbheek@centricular.com>
Thu, 2 Oct 2014 08:56:08 +0000 (14:26 +0530)
committerSebastian Dröge <sebastian@centricular.com>
Thu, 2 Oct 2014 09:47:36 +0000 (12:47 +0300)
Stream headers are updated whenever ::set_caps is called, so we can't assume
they'll be valid before the message body is written out. We *can* assume that
for queued buffers, but SOUP_MEMORY_STATIC is still wrong for those.

Also, add some debug logging for stream header interactions.

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

ext/soup/gstsouphttpclientsink.c

index 190da58..784a53f 100644 (file)
@@ -445,6 +445,7 @@ gst_soup_http_client_sink_set_caps (GstBaseSink * sink, GstCaps * caps)
   const GValue *value_array;
   int i, n;
 
+  GST_DEBUG_OBJECT (souphttpsink, "new stream headers set");
   structure = gst_caps_get_structure (caps, 0);
   value_array = gst_structure_get_value (structure, "streamheader");
   if (value_array) {
@@ -653,10 +654,13 @@ send_message_locked (GstSoupHttpClientSink * souphttpsink)
       GstBuffer *buffer = g->data;
       GstMapInfo map;
 
-      /* FIXME, lifetime of the buffer? */
+      GST_DEBUG_OBJECT (souphttpsink, "queueing stream headers");
       gst_buffer_map (buffer, &map, GST_MAP_READ);
+      /* Stream headers are updated whenever ::set_caps is called, so there's
+       * no guarantees about their lifetime and we ask libsoup to copy them 
+       * into the message body with SOUP_MEMORY_COPY. */
       soup_message_body_append (souphttpsink->message->request_body,
-          SOUP_MEMORY_STATIC, map.data, map.size);
+          SOUP_MEMORY_COPY, map.data, map.size);
       n += map.size;
       gst_buffer_unmap (buffer, &map);
     }
@@ -667,10 +671,13 @@ send_message_locked (GstSoupHttpClientSink * souphttpsink)
     if (!GST_BUFFER_FLAG_IS_SET (buffer, GST_BUFFER_FLAG_HEADER)) {
       GstMapInfo map;
 
-      /* FIXME, lifetime of the buffer? */
       gst_buffer_map (buffer, &map, GST_MAP_READ);
+      /* Queued buffers are only freed in the next iteration of the mainloop
+       * after the message body has been written out, so we don't need libsoup
+       * to copy those while appending to the body. However, if the buffer is
+       * used elsewhere, it should be copied. Hence, SOUP_MEMORY_TEMPORARY. */
       soup_message_body_append (souphttpsink->message->request_body,
-          SOUP_MEMORY_STATIC, map.data, map.size);
+          SOUP_MEMORY_TEMPORARY, map.data, map.size);
       n += map.size;
       gst_buffer_unmap (buffer, &map);
     }