GNOME Bugzilla – Bug 778437
icydemux: reset tags on empty StreamTitle value
Last modified: 2017-02-19 10:56:01 UTC
Created attachment 345414 [details] [review] gsticydemux: reset tags on empty value Some radio streams use StreamTitle='' to indicate that the previous track has stopped playing, for example while the radio host talks between tracks or during news segments. Currently no tag changes are send in this case. I have attached a patch that changes this behavior such that an empty tag object is always send if a StreamTitle or StreamUrl with an empty value is received. The patch also includes changes to the existing test to verify the changes.
Comment on attachment 345414 [details] [review] gsticydemux: reset tags on empty value Thansk for the patch, but this does not compile (you might also want to just use a boolean for the tags_found): CC libgsticydemux_la-gsticydemux.lo gsticydemux.c: In function ‘gst_icydemux_parse_and_send_tags’: gsticydemux.c:343:7: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] char *title = gst_icydemux_unicodify (strings[i] + 13); ^~~~ gsticydemux.c:352:7: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] char *url = gst_icydemux_unicodify (strings[i] + 11); ^~~~ ggsticydemux.c: In function ‘gst_icydemux_chain’: gsticydemux.c:367:6: error: ‘tags_found’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if (tags_found) ^ gsticydemux.c:325:18: note: ‘tags_found’ was declared here int length, i, tags_found; ^~~~~~~~~~
Hi Sebastian, Thanks for your quick feedback. I just read that code should be compatible with ANSI C89 (https://gstreamer.freedesktop.org/documentation/frequently-asked-questions/developing.html), but I'm getting a lot of compile errors if I specify CLANG=-std=c89, for example: ../subprojects/gstreamer/gst/printf/printf-parse.c: In function ‘__gst_printf_parse’: ../subprojects/gstreamer/gst/printf/printf-parse.c:260:15: error: C++ style comments are not allowed in ISO C90 //flags = 32; I use gst-build to compile the code: $ CFLAGS=-std=c89 meson build --buildtype=plain $ ninja -C build Am I only supposed to compile part of the code with c89? Sorry for bringing build problems into the bug report.
Don't use -std=c89 for anything really. The problem here is that the meson build does not include all the compiler warning flags that are part of the autotools build. But what you only need to do here is to move the 2 variable declarations at the beginning of the blog, and ensure that tags_found is always initialized (i.e. initialize it to 0 at the very top).
Created attachment 345602 [details] [review] Updated patch
Right, I would just have preferred to verify the changes locally before sharing it here. But I have updated the patch as requested, hopefully it will build now. From your description of the meson build it sounds like it would be better to use autotools for now. Thanks
Adding those compiler warning flags to meson is on the todo list already, just has to be done. Apart from that using meson should be fine at this point. commit 1184429e21f8c48e435625f46e3e9a79a4662498 Author: Søren Juul <zpon.dk@gmail.com> Date: Fri Feb 10 10:53:05 2017 +0100 icydemux: reset tags on empty value Some radio streams uses StreamTitle='' to reset the title after a track stopped playing, e.g. while the host talks between tracks or during news segments. This change forces an empty tag object to be distributed if StreamTitle or StreamUrl is received with empty value, thus allowing downstream elements to get notified about this. https://bugzilla.gnome.org/show_bug.cgi?id=778437