[ lame-Bugs-3463197 ] 3.99.x problem WFED and PCST frames

Bugs item #3463197, was opened at 2011-12-20 21:23
Message generated for change (Settings changed) made by robert
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100290&aid=3463197&group_id=290

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Libraries
Group: Usability
>Status: Closed
Resolution: Fixed
Priority: 5
Private: No
Submitted By: Philippe (philippe44ca)
Assigned to: Robert Hegemann (robert)
Summary: 3.99.x problem WFED and PCST frames

Initial Comment:
Switching from 3.98.x to 3.99.x (great for UTF16 support), but WFED frames are not properly written anymore  (cannot be re-read by mp3tag, for example - claiming that id3v2 hear is incorrect) and PCST (itunes PODCAST) is not accepted anymore (says that PCST is not a valid frame). WIll try to have a look at the code, but this is my first dig into lame, so need to find what's happening in id3.c / id3.h
Platform Win7 64 bits

----------------------------------------------------------------------

Comment By: Robert Hegemann (robert)
Date: 2012-01-08 15:56

Message:
Thanks for your problem sample.

A fix for the divide by zero problem, the podcast tagging issues and some
other tagging issues is now in CVS (head and lame3_99 branches).

----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2012-01-02 13:22

Message:
I've uploaded a small .mp3 file that creates the problem. Command line is
as simple as lame --mp3input tiny.mp3 res.mp3. This problem does not occur
with the lame.exe pre-compiled version, it ony occurs with my C++ builder
XE2 build, so you probably should not bother too much with it. I've tried
changing floating point and precision options and definitons a lot before
doing that patch and this does not help. Seems that either the DIV0 only
happens with C++Builder or the way lame has been built with other
compilers, the exception occurs but is managed differently

----------------------------------------------------------------------

Comment By: Robert Hegemann (robert)
Date: 2012-01-02 10:49

Message:
Can you tell more about the divide by zero in calc_noise you observed? What
LAME options do you use? Do you have a tiny sample which triggers the bug?

----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2012-01-01 18:20

Message:
Add my patch

----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2011-12-29 00:13

Message:
I've finally made 2 patches, one to allow --tv frames to badded but no T or
W frames, anything else. This is only accepted with the
--ignore-tags-errors flag. For the WFED, in fact to work (and be recognized
by itunes and mp3tag), it must be encoded as a T field, although it is a W
frame. By default, it was encoded as an ISO8859 frame as W frames should
always be, but that does not work when UTF16 is choosen for other T fields.
Last thing, I had to test for divide / 0 error in calc_noise, was causing a
fatal error. Hope this helps somebody :-)

----------------------------------------------------------------------

Comment By: Robert Hegemann (robert)
Date: 2011-12-28 02:22

Message:
Btw, adding WFED URL frames does work. The problem seems to be, APPLE
doesn't  care about ID3 definition for URL frames and expects it to be a
text frame (that's what I got after reading the info from the links you
provided). If PCST is a regular text frame, then it's quite easy to add
that to the list of text frames.

----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2011-12-28 01:42

Message:
So, after getting able to re-compile lame under C++Builder (that was a
pain, but I "used" to be BCC developer ...) I can confirm that by doing the
following change

if (isFrameIdMatching(frame_id, FRAME_ID('T', 0, 0, 0))
      ||isFrameIdMatching(frame_id, FRAME_ID('W', 0, 0, 0))
      ||isFrameIdMatching(frame_id, FRAME_ID('P', 'C', 'S', 'T')))

it adds a frame using the id3v2_add_ucs2(gfp->internal_flags, frame_id, 0,
0, text);

and that works

I have not added an entry in the MiscID enum like ID_PCST = FRAME_ID('P',
'C', 'S', 'T') b/c I think this is not a multiframe ID so it is probably
not needed to test that elsewhere 

NB : I'm wondering if it would not be "nicer" to let users play with --tv
option and just let them add frames they want (ie no test at all, rather
than doing all sort of customs in id3tag_set_textinfo_utf16. Maybe it
sounds ugly, but that would work well (sorry, I've not developper for years
and I'm new to ID3 tags structure, so maybe I'm messing things up here)

As far as WFED are concerned, I've not investigated yet but assuming they
are "almost" regular URLs Wxxx, should be easy to change - I'll see that
tomorrow

----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2011-12-27 21:26

Message:
Probably statement of the obvious when you know the code, but I had a quick
look and custom frames other than TXXX and WXXX are blocked in this
function, in id3tag.c

id3tag_set_textinfo_latin1(lame_global_flags * gfp, char const *id, char
const *text)

BTW, when force to use latin1 encoding --id3v2-latin1, lame ends with a
fatal error (32 and 64 bits versions)


----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2011-12-23 22:30

Message:
These threads might help 
http://forums.whirlpool.net.au/archive/928786
https://github.com/judwhite/IdSharp/pull/1

----------------------------------------------------------------------

Comment By: Philippe (philippe44ca)
Date: 2011-12-23 22:23

Message:
These are iTunes specific frames, unfortunately. Seems that they were both
supported in the version of id3tag.c previously used (I had a quick look at
the source). PCST stands for PODCAST for iTunes and seems to be a boolean.
With that set, itunes will recognize the track as a podcast. WFED stands
for PODCASTURL and is obviously the URL of the podcast. ITunes uses it to
differentiate different podcasts. Without them, it becomes impossible to
add podcast tracks to iTunes (of course unless you let itunes grab them,
but I don't do that b/c I have a complicated automated process for podcat
download + title web scraping + compression before I re-inject them to
iTunes). I will search for a description of these frames, but for example
they are recognized by apps like mp3tag

----------------------------------------------------------------------

Comment By: Robert Hegemann (robert)
Date: 2011-12-21 02:20

Message:
Hi Philippe, WFED and PCST frames aren't listed on ID3.org as version 2.3
tags. Do you have a link to where they are described? Is PCST a normal text
frame like TALB?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100290&aid=3463197&group_id=290

------------------------------------------------------------------------------
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2