GNOME Bugzilla – Bug 559643
Don't allow empty or NULL strings in taglists
Last modified: 2009-05-31 14:44:15 UTC
Totem-gstreamer currently accepts the empty string "" as title or artist. Yet, if these same fields are NULL, totem uses intelligent fallbacks. They should be treated the same. The symptom of this bug is that when playing a movie file with empty title and artist tags, " - " shows up as the title of the file (in both the window title and playlist). The xine backend does this already.
Created attachment 122145 [details] [review] Proposed patch
Perhaps a comment explaining why that code's there would be a good idea?
What sort of stream/file do you run into this with btw? I'd like to fix the GStreamer element posting those tags as well.
Created attachment 122188 [details] [review] Commented version Here's a version that adds a little note explaining why it NULLs empty strings. I also added the same note to the code in the xine backend that does the same thing. Tim-Philipp, I ran into this on a .wmv file I had. I agree it makes sense to do this at the gstreamer level too (assuming empty strings never have any special meaning). Do some gst elements already do this for totem?
Looks good to me. Please commit to trunk.
No. I'd want the GStreamer plugins to be fixed. The only one I can think of that's creating the problems is the ASF demuxer. I'd accept a patch to print out a warning when an empty metadata is put through Totem (flagging a bug in a GStreamer element).
Bastien, again, I agree that the gst plugin should be fixed as well. But doesn't it make sense to sanitize what you get from gst? Anyone can drop in plugins for it; it's not like it's either a trusted or controlled source. Would you accept a patch that does both? Sanitize and warn?
(In reply to comment #7) > Bastien, again, I agree that the gst plugin should be fixed as well. But > doesn't it make sense to sanitize what you get from gst? Anyone can drop in > plugins for it; it's not like it's either a trusted or controlled source. Yeah, and additional plugins can also make us crash. So the problem is the same, we need to trust the plugins to do the right thing. > Would you accept a patch that does both? Sanitize and warn? I'd rather the GStreamer developers added something like that to gst-launch's -t option, but if they refuse, we could add it to Totem. Could you check which plugin is causing the problem and eventually provide a test file?
Empty strings and NULL strings in taglists shouldn't be allowed in the first place, and any plugins that do that should be fixed IMHO, so let's disallow that in GStreamer: commit 30c890c7a2b8140da0ac2fd964a7023cd5fb22ca Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sat May 30 20:50:40 2009 +0100 taglists: warn if someone tries to add empty or NULL string tags to a taglist Also warn if an element or application tries to add a field with an empty string to a structure (NULL strings are still needed and allowed though) and do all those checks in the right function. Fixes #559643.