GNOME Bugzilla – Bug 703283
id3: gst_date_time_new: assertion `(month > 0 && month <= 12) || month == -1' failed with malformed TDAT frames
Last modified: 2013-07-04 23:50:46 UTC
I encounter random crashes while removing adder's branch. I got following error message: gst_date_time_new: assertion `(month > 0 && month <= 12) || month == -1' failed The detach procedure was invoked by pad probe handling EOS event that came from to branch's bin ghost pad. Then, in the same (streaming) thread I started detaching from adder. I've... 1. locked bin 2. unlinked it from adder 3. released adder's pad 4. scheduled setting bin's state to stopped in main thread Assertion failed message has happened right after lock that ensures that these operations are atomic was released. Unfortunately, I am unable to send backtrace right now.
Would it be possible to provide a backtrace and/or a test program ? Also note that latest stable is 1.0.7.
I've tweaked ulimit so hopefully next time I will get core dump. I would be pleased to use 1.0.7 but due to several reasons I am a bit locked with ubuntu 12.04 + gstreamer's PPA. Is there any chance to invoke there build of 1.0.7?
I caught the backtrace
+ Trace 232171
Created attachment 248081 [details] [review] Ignore malformed TDAT frames while parsing ID3v2 tags It seems that problem is related to parsing TDAT frames in ID3v2 tag parser. It assumes that data in the TDAT frame is in correct MMDD format. Then at some point it passes parsed values without any sanity checks to gst_date_time_new() which has assertion that causes crash. Attached patch is an attempt to solve this problem. Now Gst will ignore malformed TDAT frames.
Could you make such a tag/file available for us please? If it's at the begining, the first 500k or so might be enough (head --bytes=500k file > head)
Comment on attachment 248081 [details] [review] Ignore malformed TDAT frames while parsing ID3v2 tags Thanks for the patch, but would it be possible for you to create one in git format-patch format (that is, including author + commit message)? I think this needs more work. GstDateTime in 1.0 can express 'partial dates' (e.g. only year or only year/month), so I think instead of dropping the whole frame we should still extract the info that's there.
Created attachment 248122 [details] Test case 1 $ id3info ~/test1.mp3 *** Tag information for /home/admins/marcin/test1.mp3 === TALB (Album/Movie/Show title): Gypsy Beats & Balkan Bangers === TPE1 (Lead performer(s)/Soloist(s)): Gogol Bordello === COMM (Comments): ()[eng]: Enjoy... === TDAT (Date): 0005 === TSSE (Software/Hardware and settings used for encoding): LAME 3.90.3 / alt-preset standard === TCON (Content type): Lo-Fi === TLAN (Language(s)): eng === TPUB (Publisher): Atlantic Jaxx === TIT2 (Title/songname/content description): Start Wearing Purple === TRCK (Track number/Position in set): 9/15 === TYER (Year): 2006 === TXXX (User defined text information): (Rip date): 2006-05-30 === TXXX (User defined text information): (Source): CDDA === TXXX (User defined text information): (Ripping tool): EAC === TXXX (User defined text information): (Release type): Promo === TXXX (User defined text information): (Catalog #): JAXXCD004 === PRIV (Private frame): (unimplemented) *** mp3 info MPEG1/layer III Bitrate: 128KBps Frequency: 44KHz
Created attachment 248123 [details] Test case 2 $ id3info ~/test2.mp3 *** Tag information for /home/admins/marcin/test2.mp3 === TPE1 (Lead performer(s)/Soloist(s)): John Gotch and Pid-Duklanskiy ansambl === TDAT (Date): 0323 === TCON (Content type): Blues === TIT2 (Title/songname/content description): Ey pyatnaytsiat rokiv mam === TYER (Year): 2011 === TXXX (User defined text information): (Engineer): UNION === GEOB (General encapsulated object): (SfMarkers)[]: , 12 bytes === GEOB (General encapsulated object): (SfCDInfo)[]: , 124 bytes *** mp3 info MPEG1/layer III Bitrate: 256KBps Frequency: 44KHz
Hey Tim, I've added test cases. I disagree that it should extract partial information (only month in this case, as GstDateTime does not support only days). Assumption that month is valid if syntax of whole frame is invalid is quite optimistic and output can be confusing to the user. IMO GStreamer should be rather strict by default. Comparing this to the web browser world, IMHO GStreamer shouldn't act as IE which was trying to guess underskilled author's intention and introduced several rendering modes which in fact were causing even more trouble. It should be rather equivalent of Firefox enforcing standards. Please confirm, if you still think I should add this functionality to the patch.
Thanks for the samples. I'll have to investigate some more before deciding whether I think it should try to handle this more gracefully or not. Obviously it should definitely not crash or abort.
Any thoughts?
Works for me. Would be nice if you could provide a git format-patch formatted patch next time. Thanks for the patch and the sample! commit cd00206059a398ea889621ea284642e2208a56e1 Author: Marcin Lewandowski <marcin@saepia.net> Date: Fri Jul 5 00:47:08 2013 +0100 tag: ignore malformed ID3v2 TDAT frames Just skip them, don't cause criticals. https://bugzilla.gnome.org/show_bug.cgi?id=703283