GNOME Bugzilla – Bug 100686
oggs with id3 tags get recognized as mp3s
Last modified: 2004-12-22 21:47:04 UTC
oggvorbis files which are tagged with an id3(v2) tags are picked up as mp3's. It is ofcourse invalid to tag oggs with id3 tags, but it happens a lot and it doesn't corrupt the file. I made a small patch which should filter out most cases, besides the case where the sniffed header part contains the string 'vorbis' for some other reason (eg. being mentioned in the id3 tag). But i estimated the chance of that happening minute ;)
Created attachment 12852 [details] [review] libgnomevfs/gnome-vfs-mime-magic.c ogg as mp3 patch
I don't understand this patch. The full code is: if (strncmp ((char *) sniff_buffer->buffer, "ID3", 3) == 0 && (sniff_buffer->buffer[3] != 0xffu) && (sniff_buffer->buffer[4] != 0xffu) && (sniff_buffer->buffer[6] < 0x80u) && (sniff_buffer->buffer[7] < 0x80u) && (sniff_buffer->buffer[8] < 0x80u) && (sniff_buffer->buffer[9] < 0x80u)) { return TRUE; } ie the first 3 chars of sniff_buffer->buffer are "ID3". How can your strcasecmp((char *) sniff_buffer->buffer, "vorbis") be ever == 0 ? And didn't you invert your test anyway ? didn't you meant if (strcasecmp((char *) sniff_buffer->buffer, "vorbis") == 0) ?
You're right this won't work as planned, my C is a bit too rusty it seems i did some wrong assumptions :/ I was thinking of doing it different anyway, so here is a revised patch. Which looks for the start of a vorbis identification header. I hope this is better.
Created attachment 12870 [details] [review] libgnome-vfs/gnome-vfs-mime-magic.c corrected ogg detection patch
I think something like if (strcmp(sniff_buffer->buffer[..], "vorbis") == 0) {...} would be more readable. Thanks for the updated patch :)
Yeah it basicly does that, except for the first byte. *needs to get his c reference back, where did i leave it*
Created attachment 13697 [details] [review] more readable version of the patch
How does this patch look ? (and more importantly, is it working ?)
It works for me, it's easy to test : just add an id3 tag to a an ogg/vorbis encoded file. And yes this is more readable, i need to fix my C :/
Created attachment 13831 [details] [review] corrected short patch
the vorbis header isn't always in the same place, so the sample still needs to be searched trough. This works for me, your corrected patch doesn't (spoke a bit too quick there).
Created attachment 14410 [details] [review] New patch.
I checked in the attached patch. Can you verify it works?
It doesn't work, i remember myself trying with strstr_len at first, but it didn't work out. The only reason i can think of is that the buffer may contain chars that the function can't handle (?), but i'm probably merely showing off my lack of thorough C knowledge here ;) My last patch has been in Gentoo for quite a while now and works confirmed fine, altough i can see that it is probably not clean/fast enough to use.
Oh yeah. there are probably null-bytes in the buffer. There goes my nice fix. :) I'll change it to your patch then.