GNOME Bugzilla – Bug 334375
[id3demux] [id3v2mux] ID3 tag rewriting is lossy
Last modified: 2006-07-26 10:09:21 UTC
+++ This bug was initially created as a clone of Bug #323658 +++ ID3 tag rewriting using either the id3mux or tagid3v2mux plugins will lose valid ID3v2.4.0 tags that gstreamer doesn't explicitly have a GST_TAG_ for, for example "TCOM" (for composer) or "TENC" (for encoding). Example, using the following file (with ID3v2.3 tags): http://gstreamer.net/media/incoming/V2.mp3 Retagged using id3mux plugin using this pipeline: gst-launch-0.10 filesrc location=V2.mp3 ! id3demux ! id3mux ! filesink location=V2-id3mux.mp3 results in the file in attachment #60953 [details]. Retagged using the tagid3v2mux plugin with this pipeline gst-launch-0.10 filesrc location=V2.mp3 ! id3demux ! tagid3v2mux ! filesink location=V2-taglib.mp3 results in the file in attachment #60950 [details].
As a possible fix Christophe (teuf) and have Jan Schmidt (thaytan) have proposed adding support to id3demux to forward unknown tags as GST_TAG_UNKNOWN. Relevant conversation on #rhythmbox on IRC, teuf (Christophe) and blarney (Alex L): (02:35:28) teuf: the scheme described by thaytan for id3demux could be used by taglibmux however I think (02:35:51) blarney: what was that? (02:35:53) teuf: (taglibn seems to be able to parse an mp3 frame from a memory buffer, but not a full tag) (02:36:15) teuf: forward unknown frames as TAG_UNKNOWN tags containing binary data (02:36:21) teuf: and letting other elements handling them if they can (02:36:47) blarney: so this would mean writing a tagid3v2demux element? (02:37:10) teuf: nope, this would mean waiting for thaytan to add support for that in id3demux (02:37:15) teuf: and then handle those unknown tags in taglib (02:37:21) teuf: taglibmux I mean (02:37:24) blarney: oh, ok (02:38:11) blarney: so the same thing could be done with id3mux I guess, unless we want to drop support for it
Suggestion from Tim-Philipp Müller from bug #333501: > As I see it, there are two possible solutions to this problem: > > (a) have a separate id3remux element that specialises in modifying > ID3 tags and reads + writes them. Could be based on taglib as > well. > > (b) id3demux from gst-plugins-good could send a custom event downstream > with ID3 frames that it doesn't recognise or that can't be conveyed > via the GStreamer tags system. id3v2mux could detect those events > and 'inject' those frames again into the tag that it's going to > write. Or something like that. Surely solvable somehow that way.
Fwiw, I feel approach (a) is cleaner (and probably trivial to implement when there is a working tagid3v2demux), but approach (b) would work too. I started to work on a tagid3v2demux, the code is mostly done, but I have to figure out why taglib can't parse the id3v2 tag it's fed with.
Over to -good now that id3v2mux is there too.
(In reply to comment #4) > Over to -good now that id3v2mux is there too. Has tagid3v2mux been renamed to id3v2mux? Given that this has been switched to -good I assume that this lossless tag rewriting, if it implemented is going to be via a combination of id3demux and id3v2mux and will not include id3mux? Is there any progress on this?
Yes, it's been renamed to id3v2mux (sorry for the confusion). Lossless tag rewriting is going to be a combination of id3demux and id3v2mux. I don't think it will include the current (libid3tag-based) id3mux element in its present form, no. However, my hope is that we will get rid of the id3mux element in its current incarnation completely and replace it with something API compatible under the same name based on the taglib stuff in -good (that could be just id3v2mux registering itself under an additional element name or a bin containing id3v2mux and a new id3v1mux or something, who knows). No progress on this yet, but now that the taglib plugin is in -good things should be much easier to do. Hopefully we might get this into the next -good release (after the pending one).
See bug #347091 for a solution for OGG vorbis tags.
Step 1: 2006-07-23 Tim-Philipp Müller <tim at centricular dot net> * gst/id3demux/id3tags.c: (id3demux_add_id3v2_frame_blob_to_taglist), (id3demux_id3v2_frames_to_tag_list): Put ID3v2 frames we can't parse as binary blobs into private tags, so that they are not lost when retagging, at least once id3v2mux has been taught to re-inject those frames again. See bug #334375.
Created attachment 69560 [details] [review] Partial patch handles re-encoding via taglib This is a partial patch handling the re-encoding the binary blob via taglib. Tim is working on the other part which is to handle multiple binary tags (including binary blobs). This current patch will only re-encode the first binary blob it finds.
Created attachment 69596 [details] [review] Make id3v2mux handle multiple tags of the same type and inject ID3v2 frames from blobs Patch against -good CVS. Rewrites id3v2mux tag render logic to handle multiple occurrences of the major tags that can occur multiple times; also makes id3v2mux re-inject unhandled ID3v2 frames from binary blobs as extracted by id3demux from CVS (frames will be converted by taglib to ID3v2.4.0 format though, there's not much we can do about that).
Created attachment 69623 [details] [review] Fix handling of comments (COMM) Comments use the COMM tag, not TCOM (which is composer). I added a new method add_comment_tag() because they need to use the handling using the special setComment() method. A setText() on a generic TextIdentificationFrame doesn't work properly.
Created attachment 69624 [details] [review] Minor fix to not pass tag name to add_comment_tag()
Great, thanks for testing. Committed my original version: 2006-07-26 Tim-Philipp Müller <tim at centricular dot net> * ext/taglib/gstid3v2mux.cc: Handle multiple tags of the same type properly. Re-inject unparsed ID3v2 frames that we get as binary blobs from id3demux into the tag again so we don't lose information when retagging (#334375). You're right about the comment tag handling, that needs to be fixed. I would prefer to keep separate comments as separate strings/frames though instead of just concatenating all the comment strings into one string, if that's possible. Will attach possible patch for that in a second.
Created attachment 69648 [details] [review] better comments handling
Works for me, modulo not preserving some iTunes specific metadata.
Committed this one as well: 2006-07-26 Tim-Philipp Müller <tim at centricular dot net> * ext/taglib/gstid3v2mux.cc: Fix writing of comment frames (should be COMM not TCOM), is still sub-optimal though, since we don't retain or extract the comment descriptions properly (#334375, also see #334375). I think this bug can be closed now. I've opened bug #348762 for the additional comments handling improvements needed.