After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 407349 - [id3demux] wrongly interprets TDAT as year
[id3demux] wrongly interprets TDAT as year
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.3
Other Linux
: Normal normal
: 0.10.6
Assigned To: Tim-Philipp Müller
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-02-13 04:59 UTC by Sam D. Chuparkoff
Modified: 2007-03-07 10:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
id3v2 with tyer before tdat (6.00 KB, audio/mpeg)
2007-02-13 05:01 UTC, Sam D. Chuparkoff
  Details
id3v2 with tdat before tyer (6.00 KB, audio/mpeg)
2007-02-13 05:01 UTC, Sam D. Chuparkoff
  Details
Patch to remove errant tdat conversion (832 bytes, patch)
2007-03-01 07:32 UTC, Sam D. Chuparkoff
none Details | Review

Description Sam D. Chuparkoff 2007-02-13 04:59:20 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 ...
Comment 1 Sam D. Chuparkoff 2007-02-13 05:01:18 UTC
Created attachment 82436 [details]
id3v2 with tyer before tdat
Comment 2 Sam D. Chuparkoff 2007-02-13 05:01:51 UTC
Created attachment 82437 [details]
id3v2 with tdat before tyer
Comment 3 Jan Schmidt 2007-02-15 16:13:02 UTC
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.
Comment 4 Sam D. Chuparkoff 2007-03-01 07:32:39 UTC
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.
Comment 5 Tim-Philipp Müller 2007-03-06 23:24:20 UTC
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.
Comment 6 Jan Schmidt 2007-03-07 10:58:15 UTC
You rock, Tim.