[1.7.7] bug in FLAC write support (remove_other_tags) 2

[nitro322]

Ok, after some additional testing I have to report that this is still an issue. Take the following example song with tags:

comment[0]: ARTIST=Dream Theater
comment[1]: ALBUM=Octavarium
comment[2]: TITLE=I Walk Beside You
comment[3]: DATE=2005
comment[4]: TRACKNUMBER=04
comment[5]: GENRE=Progressive

I'm using this example because I think it does a better job of illustrating my point than a simple description.

Now, there are two scenarios that I generally want to carry out when working with tags: update/modify existing tags, or replace wholesale replace the tags with new information. The latter is easy to accomplish, but the former seems to be impossible because of getID3()'s implementation of the overwrite_tags and remove_other_tags options. To illustrate, I wrote a very small program that sets the TITLE tag, then ran the program with those two options set to the four different possible combinations and captured the results after each run.

For completeness, here's the program:

Code: Select all

require_once('getid3/getid3.php');
$file = '04-I Walk Beside You.flac';
$getID3 = new getID3;
getid3_lib::IncludeDependency(GETID3_INCLUDEPATH.'write.php', __FILE__);
$TaggingFormat = 'UTF-8';
$getID3->setOption(array('encoding'=>$TaggingFormat));
$tagwriter = new getid3_writetags;
$tagwriter->filename = $file;
$tagwriter->tagformats = array('metaflac');
$tagwriter->tag_encoding   = $TaggingFormat;
$tagwriter->overwrite_tags = false;
$tagwriter->remove_other_tags = false;
$TagData['TITLE'][] = 'Test Title';
$tagwriter->tag_data = $TagData;
$tagwriter->WriteTags();
$fileinfo = $getID3->analyze($file);
getid3_lib::CopyTagsToComments($fileinfo);
echo "overwrite_tags = false\n";
echo "remove_other_tags = false\n\n";
foreach ($fileinfo['comments'] as $tagname => $tagval)
    for ($i = 0; $i < count($tagval); $i++)
        echo strtoupper($tagname)." = {$tagval[$i]}\n";

Next up the output for the various combinations of true/false settings for overwrite_tags and remove_other_tags

Code: Select all

overwrite_tags = false
remove_other_tags = false

TITLE = Test Title
TITLE = I Walk Beside You
ARTIST = Dream Theater
ALBUM = Octavarium
DATE = 2005
TRACKNUMBER = 04
GENRE = Progressive

Code: Select all

overwrite_tags = true
remove_other_tags = false

TITLE = Test Title

Code: Select all

overwrite_tags = false
remove_other_tags = true

TITLE = Test Title
TITLE = I Walk Beside You
ARTIST = Dream Theater
ALBUM = Octavarium
DATE = 2005
TRACKNUMBER = 04
GENRE = Progressive

Code: Select all

overwrite_tags = true
remove_other_tags = true

TITLE = Test Title

As indicated in the last post, remove_other_tags is completely ignored when dealing with FLAC files. I would have to assume that this same behavior applies to Ogg Vorbis as well. Instead, only overwrite_tags seems to have any effect, and I believe it is not properly implemented as it greatly reduces the amount of flexability to users.

Going back to my scenario at the beginning of this post, it is apparently impossible to do something as simple as replacing the TITLE of the track without either creating a duplicate entry or blowing away all other tags. Sure, I could accomplish this with additional PHP code that backs up the currents tags to an array, update the array, then write them back, but all of that extra work essentially defeats the purpose using getID3() to begin with.

The behavior that I propose, and what I originally expected to see based on the option names, is as follows: (NOTE: this is my mockup, not actual output)

Code: Select all

overwrite_tags = false
remove_other_tags = false

TITLE = Test Title
TITLE = I Walk Beside You
ARTIST = Dream Theater
ALBUM = Octavarium
DATE = 2005
TRACKNUMBER = 04
GENRE = Progressive

Code: Select all

overwrite_tags = true
remove_other_tags = false

TITLE = Test Title
ARTIST = Dream Theater
ALBUM = Octavarium
DATE = 2005
TRACKNUMBER = 04
GENRE = Progressive

Code: Select all

overwrite_tags = false
remove_other_tags = true

TITLE = Test Title
TITLE = I Walk Beside You

Code: Select all

overwrite_tags = true
remove_other_tags = true

TITLE = Test Title

To me, this is a far more useful and intuitive bahavior. Does anyone else, particularly developers, have an opinion on this? I'd really love to read your feedback. Thanks.

[Allan Hansen]

I think the problem is, that remove_other_tags does what it says, while overwrite_tags is misnamed and should be reallly be called overwrite_comments or overwrite_tagdata.

ARTIST, TITLE, etc are not tags. They are comments/tagdata. A collection of comments make a tag.

FLAC files can have three different tags, although one is normal. The three different tags are FLACtag, ID3v1 and ID3v2.

Likewise, a MP3 file can have three different tags, although 1-2 is normal: APEtag, ID3v1 and ID3v2.

So back to your FLAC file. It it has an ID3 tag and you update the file with remove_other_tags=false, the ID3 tag should remain intact, while remove_other_tags=true should delete the ID3 tag.

I hope this explains it?

You are however asking for an opinion. I did not write any of the tagwriting code and have never used it. I do not have an opinion. I will however soon begin to either rewrite or convert existing code into getID3() 2.0.0.

[nitro322]

Allan, thanks for your feedback. I guess the fundamental problem here is that getID3() treats FLAC, Vorbis, etc. comments as a single tag (which I guess would be technically correct given your explanation), whereas from an end-user perspective they appear to be just as separate as ID3 tags. While I believe getID3() should act in a consistant manner from the end-user's perspective, that's really a matter of personal opinion. I didn't see any other mention of this in the forums, so maybe I'm the only one that does feel that way. :-)

One question for you, if you don't mind. Given my example above, do you know of any straightforward way to change a single existing comment while leaving all other comments in tact? Eg, the true/true example in my mockup output. This will be one of the primary uses for the script I'm writing, and if I have to code all of this directly in PHP then it'd probably be easier to work with the files directly rather than using getID3().

[Allan Hansen]

Allan, thanks for your feedback. I guess the fundamental problem here is that getID3() treats FLAC, Vorbis, etc. comments as a single tag (which I guess would be technically correct given your explanation), whereas from an end-user perspective they appear to be just as separate as ID3 tags.

An MP3 file can have no more than two ID3 tags: One ID3v1 tag and one ID3v2 tag. ID3 tags can contain several comments.
ARTIST is not a tag. ALBUM is not a tag. Both are fields that have a corresponsing value. A field/value pair is called a comment or tagdata -- at least in our terminology.

One question for you, if you don't mind. Given my example above, do you know of any straightforward way to change a single existing comment while leaving all other comments in tact?

There are two ways.

1. Read the comments with getID3(). Change the desired value. Write tag with getID3().

2. System call to metaflac

`metaflac --remove-tag=ARTIST`;
`metaflac --set-tag=ARTIST "Alice Cooper"`;

it is possible that

`metaflac --remove-tag=ARTIST --set-tag=ARTIST "Alice Cooper"`;

works as well.


I recommend the first method, as you might encounter trouble with UTF-8 encoded characters with the second.

[nitro322]

Thanks again for the response, Allan. I should have a chance to work on this more over the weekend, and I'll definitely try your suggestions.