[Bug 659573] multipartdemux: segfault

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=659573
  GStreamer | gst-plugins-good | git

Nicolas Dufresne <nicolas.dufresne> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #197135|none                        |reviewed
             status|                            |

--- Comment #16 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2014-06-10 21:35:15 UTC ---
Review of attachment 197135:
 --> (https://bugzilla.gnome.org/review?bug=659573&attachment=197135)

Minor comment, feel free to merge when/if you are happy with it. Content-type
seems mandatory, so skipping or failing seems right. I didn't found anything in
RFC 2046 that would allow having two streams with the same content type. This
means it would have to be implement with a GStreamer extension, e.g.
X-gstreamer-stream-id: <stream-id>. We could do that eventually if really
needed.

::: gst/multipart/multipartdemux.c
@@ +349,3 @@
+  } else {
+    GST_WARNING_OBJECT (demux, "No MIME type, cannot create pad");
+    return NULL;

Maybe I miss something, or code have changed, but it seems like NULL isn't
handled have calling gst_multipart_find_pad_by_mime(). At the same time, it
will never be called with the following fix. So maybe we should put
g_return_val_if_fail(), and do that at both place, as a crash avoidance, rather
then handling it and crashing later ?

@@ +590,3 @@
+    } else if (G_UNLIKELY (!multipart->mime_type)) {
+      GST_DEBUG_OBJECT (multipart, "content has no MIME type.");
+      gst_adapter_flush (adapter, size - datalen);

That looks good.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.