Patches for timePerFrame, RVA2, empty Xiph comments

[Stephen F. Booth]

Hello TagLib developers,

I submitted some patches a while back to the bug reporter and they  
are still unconfirmed.  Since these are bug fixes with patches  
included, I decided to send them to this list in the homes of having  
them committed to svn.

The first patch fixes the calculation of timePerFrame in  
mpegproperties.cpp (http://bugs.kde.org/show_bug.cgi?id=143938):

The calculation of timePerFrame in mepegproperties.cpp almost always  
results in zero because the result of integer division is being cast  
to a double.  This leads to the length() method of MPEGFile return  
0.  It is necessary to promote one of the ints before dividing to get  
an accurate result.  Here is a patch with one way to fix the problem:

svn diff mpegproperties.cpp
Index: mpegproperties.cpp
===================================================================
--- mpegproperties.cpp  (revision 651409)
+++ mpegproperties.cpp  (working copy)
@@ -213,7 +213,7 @@
    {
        static const int blockSize[] = { 0, 384, 1152, 1152 };

-      double timePerFrame = blockSize[firstHeader.layer()] /  
firstHeader.sampleRate();
+      double timePerFrame = (double)blockSize[firstHeader.layer()] /  
firstHeader.sampleRate();
        d->length = int(timePerFrame * d->xingHeader->totalFrames());
        d->bitrate = d->length > 0 ? d->xingHeader->totalSize() * 8 /  
d->length / 1000 : 0;
    }


The second patch fixes the gain calculation in RVA2 MP3 frames  
(http://bugs.kde.org/show_bug.cgi?id=145298):

The gain adjustment is handled incorrectly when set (it is currently  
divided by 512 and should be multiplied), and the tag is parsed  
incorrectly because the separator length is not accounted for.  The  
following patch fixes both issues:

Index: relativevolumeframe.cpp
===================================================================
--- relativevolumeframe.cpp     (revision 651409)
+++ relativevolumeframe.cpp     (working copy)
@@ -130,7 +130,7 @@

  void RelativeVolumeFrame::setVolumeAdjustment(float adjustment,  
ChannelType type)
  {
-  d->channels[type].volumeAdjustment = short(adjustment / float(512));
+  d->channels[type].volumeAdjustment = short(adjustment * float(512));
  }

  void RelativeVolumeFrame::setVolumeAdjustment(float adjustment)
@@ -164,8 +164,10 @@

  void RelativeVolumeFrame::parseFields(const ByteVector &data)
  {
-  uint pos = data.find(textDelimiter(String::Latin1));
+  ByteVector delimiter = textDelimiter(String::Latin1);
+  uint pos = data.find(delimiter);
    d->identification = String(data.mid(0, pos), String::Latin1);
+  pos += delimiter.size();

    // Each channel is at least 4 bytes.

The third patch changes the behavior of Ogg::XiphComment::addField to  
behave as documented (http://bugs.kde.org/show_bug.cgi?id=145532):

The documentation for Ogg::XiphComment::addField says "If the field  
value is empty, the field will be removed."  This is implemented  
incorrectly; this patch fixes the behavior to match the documentation.

Index: xiphcomment.cpp
===================================================================
--- xiphcomment.cpp     (revision 651409)
+++ xiphcomment.cpp     (working copy)
@@ -188,7 +188,7 @@
    if(replace)
      removeField(key.upper());

-  if(!key.isEmpty())
+  if(!value.isEmpty())
      d->fieldListMap[key.upper()].append(value);
  }


Thanks for considering these.

Stephen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2409 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/taglib-devel/attachments/20070527/c531284b/attachment.bin