After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 783307 - rtspsrc: uniquify stream ids
rtspsrc: uniquify stream ids
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 1.13.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-01 00:16 UTC by Mathieu Duponchelle
Modified: 2017-06-16 15:31 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
rtspsrc: uniquify stream ids (4.18 KB, patch)
2017-06-01 00:16 UTC, Mathieu Duponchelle 		needs-work Details | Review
rtspsrc: do not checksum the stream id (1.18 KB, patch)
2017-06-15 17:10 UTC, Mathieu Duponchelle 		committed Details | Review

Description Mathieu Duponchelle 2017-06-01 00:16:42 UTC 
Following up on https://bugzilla.gnome.org/show_bug.cgi?id=783261,
this is an alternate proposal for providing unique and stable
stream ids for a given rtsp source.
Comment 1 Mathieu Duponchelle 2017-06-01 00:16:45 UTC 
Created attachment 352976 [details] [review]
rtspsrc: uniquify stream ids
Comment 2 Thibault Saunier 2017-06-07 21:11:29 UTC 
Review of attachment 352976 [details] [review]:

That should work indeed. (please answer the question still :)

::: gst/rtsp/gstrtspsrc.c
@@ +1509,3 @@
+  gchar *stream_id = g_strdup_printf ("%s%d%d%s%d", media->media, media->port,
+      media->num_ports, media->proto, stream->default_pt);
+  gchar *res = g_compute_checksum_for_string (G_CHECKSUM_MD5, stream_id, -1);

Why do you make a checksum of it?
Comment 3 Mathieu Duponchelle 2017-06-07 21:19:07 UTC 
Review of attachment 352976 [details] [review]:

::: gst/rtsp/gstrtspsrc.c
@@ +1509,3 @@
+  gchar *stream_id = g_strdup_printf ("%s%d%d%s%d", media->media, media->port,
+      media->num_ports, media->proto, stream->default_pt);
+  gchar *res = g_compute_checksum_for_string (G_CHECKSUM_MD5, stream_id, -1);

Because some of these elements may contain forward slashes (proto in particular)
Comment 4 Mathieu Duponchelle 2017-06-07 21:30:31 UTC 
Attachment 352976 [details] pushed as f6283b0 - rtspsrc: uniquify stream ids
Comment 5 Sebastian Dröge (slomo) 2017-06-07 21:45:52 UTC 
Review of attachment 352976 [details] [review]:

::: gst/rtsp/gstrtspsrc.c
@@ +1509,3 @@
+  gchar *stream_id = g_strdup_printf ("%s%d%d%s%d", media->media, media->port,
+      media->num_ports, media->proto, stream->default_pt);
+  gchar *res = g_compute_checksum_for_string (G_CHECKSUM_MD5, stream_id, -1);

That's not really a reason, please don't use a checksum here as it makes debugging harder. There's nothing disallowing usage of / in stream IDs, but feel free to just replace / with some other character if it annoys you too much.
Comment 6 Sebastian Dröge (slomo) 2017-06-09 07:01:03 UTC 
Mathieu?
Comment 7 Mathieu Duponchelle 2017-06-09 15:54:28 UTC 
OK I'll change that, I don't really see how it would make debugging any harder though ?
Comment 8 Sebastian Dröge (slomo) 2017-06-12 06:38:42 UTC 
If you see some random hash in the stream-id, you have no idea where that actually comes from. Otherwise you can reconstruct which RTSP stream you actually have there easily.
Comment 9 Mathieu Duponchelle 2017-06-15 17:10:03 UTC 
Created attachment 353849 [details] [review]
rtspsrc: do not checksum the stream id
Comment 10 Thibault Saunier 2017-06-15 18:39:05 UTC 
Review of attachment 353849 [details] [review]:

Go?
Comment 11 Sebastian Dröge (slomo) 2017-06-15 21:07:31 UTC 
Looks good to me
Comment 12 Mathieu Duponchelle 2017-06-16 15:30:31 UTC 
Comment on attachment 353849 [details] [review]
rtspsrc: do not checksum the stream id

Attachment 353849 [details] pushed as 0da5679 - rtspsrc: do not checksum the stream id