GNOME Bugzilla – Bug 407349
[id3demux] wrongly interprets TDAT as year
Last modified: 2007-03-07 10:58:15 UTC
Seems id3demux naively converts TDAT fields to TDRC, with the effect that the value of TDAT is interpreted as a year. This is only noticeable when the TDAT tag precedes the TYER field in the source file. I'll attach a couple of files to demonstrate this. This case is fine. $ id3info tyer-before-tdat.mp3 *** Tag information for tyer-before-tdat.mp3 === TYER (Year): 1977 === TDAT (Date): 2306 *** mp3 info MPEG1/layer III Bitrate: 64KBps Frequency: 44KHz $ gst-launch-0.10 filesrc location=tyer-before-tdat.mp3 ! id3demux ! \ fakesink -t Setting pipeline to PAUSED ... Pipeline is PREROLLING ... FOUND TAG : found by element "id3demux0". date: 1977-01-01 Pipeline is PREROLLED ... Setting pipeline to PLAYING ... New clock: GstSystemClock Got EOS from element "pipeline0". Execution ended after 950000 ns. Setting pipeline to PAUSED ... Setting pipeline to READY ... Setting pipeline to NULL ... FREEING pipeline ... I'd expect to get the same answer in this case, but I don't. $ id3info tdat-before-tyer.mp3 *** Tag information for tdat-before-tyer.mp3 === TDAT (Date): 2306 === TYER (Year): 1977 *** mp3 info MPEG1/layer III Bitrate: 64KBps Frequency: 44KHz $ gst-launch-0.10 filesrc location=tdat-before-tyer.mp3 ! id3demux ! \ fakesink -t Setting pipeline to PAUSED ... Pipeline is PREROLLING ... FOUND TAG : found by element "id3demux0". date: 2306-01-01 Pipeline is PREROLLED ... Setting pipeline to PLAYING ... New clock: GstSystemClock Got EOS from element "pipeline0". Execution ended after 880000 ns. Setting pipeline to PAUSED ... Setting pipeline to READY ... Setting pipeline to NULL ... FREEING pipeline ...
Created attachment 82436 [details] id3v2 with tyer before tdat
Created attachment 82437 [details] id3v2 with tdat before tyer
Hrmn. id3demux isn't too well designed for this case - there's no nice way to handle tags from older id3v2 specs that require conversion to an equivalent v2.4 format. Special casing the handling in the text-tag parsing seems the best way.
Created attachment 83621 [details] [review] Patch to remove errant tdat conversion If someone wants to just fix this so that TYER, TDAT, and TIME are used to create a TDRC, that's cool. Otherwise, here's the obvious patch that removes the errant TDAT conversion.
Should be fixed now: 2007-03-06 Tim-Philipp Müller <tim at centricular dot net> * gst/id3demux/id3tags.c: (id3demux_id3v2_frames_to_tag_list): * gst/id3demux/id3tags.h: * gst/id3demux/id3v2frames.c: (id3demux_id3v2_parse_frame), (parse_obsolete_tdat_frame): Do not convert obsolete TDA/TDAT frames to TDRC frames, otherwise the four-digit number will be interpreted as a year, whereas it is month and day in DDMM format. Instead, parse TDAT frames and fix up the date in the GST_TAG_DATE tag later if we also extracted a year. Fixes #407349. 2007-03-06 Tim-Philipp Müller <tim at centricular dot net> * configure.ac: * tests/files/Makefile.am: * tests/files/id3-407349-1.tag: * tests/files/id3-407349-2.tag: Add directory where data for unit tests can be stored. * tests/Makefile.am: * tests/check/Makefile.am: * tests/check/elements/.cvsignore: * tests/check/elements/id3demux.c: (pad_added_cb), (error_cb), (read_tags_from_file), (run_check_for_file), (check_date_1977_06_23), (GST_START_TEST), (id3demux_suite): Add unit test for id3demux, and in particular for bug #407349. Only testing pull-mode for now; push mode doesn't work yet because the test files are smaller than ID3_TYPE_FIND_MIN_SIZE.
You rock, Tim.