Bug #55120 for MP3-Tag: Cannot set genre to Blues or (0)

Mon Mar 01 19:52:17 2010 eric_a_benson [...] yahoo.com - Ticket created

Subject:	Cannot set genre to Blues or (0)
Date:	Mon, 1 Mar 2010 16:51:33 -0800 (PST)
To:	bug-MP3-Tag [...] rt.cpan.org
From:	Eric Benson <eric_a_benson [...] yahoo.com>

Using MP3-Tag-1.12 with Perl 5.10.1, I cannot create a TCON frame with the string "Blues" or "(0)" as the value. It gets converted into a numeric value "0", which is interpreted as the genre with the name "0" rather than the ID3 genre numbered 0, which is Blues. I believe this is caused by a bug in the function MP3::Tag::Implemenation::_massage_genres. The line that reads $genre = "($genre)" if $genre and not $genre =~ /\D/; # Only digits should instead just read $genre = "($genre)" unless $genre =~ /\D/; # Only digits I believe this is always correct because $genre cannot be undefined or an empty string at this point, and if it is the number 0 or the string "0" then it should be considered the ID3 genre 0, or Blues.

Mon Mar 01 20:42:45 2010 nospam-abuse [...] ilyaz.org - Correspondence added

CC:	undisclosed-recipients: ;
Subject:	Re: [rt.cpan.org #55120] Cannot set genre to Blues or (0)
Date:	Mon, 1 Mar 2010 17:42:13 -0800
To:	Eric Benson via RT <bug-MP3-Tag [...] rt.cpan.org>
From:	Ilya Zakharevich <nospam-abuse [...] ilyaz.org>

On Mon, Mar 01, 2010 at 07:52:18PM -0500, Eric Benson via RT wrote: Show quoted text

> Using MP3-Tag-1.12 with Perl 5.10.1, I cannot create a TCON frame with the string "Blues" or "(0)" as the value. It gets converted into a numeric value "0", which is interpreted as the genre with the name "0" rather than the ID3 genre numbered 0, which is Blues. I believe this is caused by a bug in the function MP3::Tag::Implemenation::_massage_genres. The line that reads > > $genre = "($genre)" if $genre and not $genre =~ /\D/; # Only digits > > should instead just read > > $genre = "($genre)" unless $genre =~ /\D/; # Only digits > > I believe this is always correct because $genre cannot be undefined or an empty string at this point, and if it is the number 0 or the string "0" then it should be considered the ID3 genre 0, or Blues.

I see that $genre is always defined in this case. I do not see why it cannot be an empty string... (This is "semantically impossible", but I do not see which way it is excluded by the programming logic...) What we want to do: a) keep empty string empty; b) put a number in parens; c) keep non-empty non-number. !/\D/ is true on numbers and an empty string. So I'm changing it to $genre = "($genre)" if length $genre and not $genre =~ /\D/; # Only digits Does it fix things on your end? Thanks for a very clear bug report, Ilya

Mon Mar 01 20:42:46 2010 The RT System itself - Status changed from 'new' to 'open'

Tue Mar 02 10:05:39 2010 eric_a_benson [...] yahoo.com - Correspondence added

Subject:	Re: [rt.cpan.org #55120] Cannot set genre to Blues or (0)
Date:	Tue, 2 Mar 2010 07:04:23 -0800 (PST)
To:	bug-MP3-Tag [...] rt.cpan.org
From:	Eric Benson <eric_a_benson [...] yahoo.com>

Show quoted text

> I see that $genre is always defined in this case. I do not see why it > cannot be an empty string... (This is "semantically impossible", but > I do not see which way it is excluded by the programming logic...)

Actually it is excluded by program logic. The first line of the foreach loop is next if $genre eq ""; # (12)(13) ==> in front, and between which eliminates the possibility of an empty string later in the loop. Show quoted text

> $genre = "($genre)" if length $genre and not $genre =~ /\D/; # Only digits

Show quoted text

> Does it fix things on your end?

Yes, this has the same effect, but as I said it is redundant. If you follow the logic through that foreach loop you will see that it is in fact impossible for $genre to be an empty string at this point. You may prefer this solution if you want to allow for the code to change in the future and permit an empty string to be used as a genre, but that does seem unlikely. Show quoted text

> Thanks for a very clear bug report, > Ilya

Thanks for a quick response, Eric

Fri Apr 09 16:03:57 2010 eric_a_benson [...] yahoo.com - Correspondence added

Subject:	Re: [rt.cpan.org #55120] Cannot set genre to Blues or (0)
Date:	Fri, 9 Apr 2010 13:03:42 -0700 (PDT)
To:	bug-MP3-Tag [...] rt.cpan.org
From:	Eric Benson <eric_a_benson [...] yahoo.com>

Are you planning to release a new version of the module with this fix?

Fri Apr 09 17:30:58 2010 nospam-abuse [...] ilyaz.org - Correspondence added

CC:	undisclosed-recipients: ;
Subject:	Re: [rt.cpan.org #55120] Cannot set genre to Blues or (0)
Date:	Fri, 9 Apr 2010 14:30:41 -0700
To:	Eric Benson via RT <bug-MP3-Tag [...] rt.cpan.org>
From:	Ilya Zakharevich <nospam-abuse [...] ilyaz.org>

On Fri, Apr 09, 2010 at 04:03:57PM -0400, Eric Benson via RT wrote: Show quoted text

> Queue: MP3-Tag > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=55120 > > > Are you planning to release a new version of the module with this fix?

Of course - but as usual, no ETA is possible. Yours, Ilya

Tue Jul 13 10:12:38 2010 nospam-abuse [...] ilyaz.org - Correspondence added

Subject:	Re: [rt.cpan.org #55120] Cannot set genre to Blues or (0)
Date:	Tue, 13 Jul 2010 07:12:25 -0700
To:	Eric Benson via RT <bug-MP3-Tag [...] rt.cpan.org>
From:	Ilya Zakharevich <nospam-abuse [...] ilyaz.org>

On Fri, Apr 09, 2010 at 04:03:57PM -0400, Eric Benson via RT wrote: Show quoted text

> Queue: MP3-Tag > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=55120 > > > Are you planning to release a new version of the module with this fix?

I put on PAUSE. Should propagate to CPAN soon. Thanks for your contributions, Ilya