Bug 122698 - Saving APE tags in mp3 file adds unwanted padding
Summary: Saving APE tags in mp3 file adds unwanted padding
Status: RESOLVED FIXED
Alias: None
Product: taglib
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Scott Wheeler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-25 15:27 UTC by Matej Vadnjal
Modified: 2007-07-18 18:41 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Proposed patch (1.19 KB, patch)
2006-02-26 13:48 UTC, Matej Vadnjal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matej Vadnjal 2006-02-25 15:27:56 UTC
Version:           1.4 (using KDE KDE 3.5.1)
Installed from:    Gentoo Packages
OS:                Linux

When saving an APE tag to an mp3 file, which allready has an ID3v1 tag, taglib adds 896 empty bytes to the end of the file. However the APE tag is correctly inserted right in front if the ID3v1 tag. 

It looks like this is a problem in the insert() function which uses bufferSize() to insert 1024 bytes of padding after the APE tag. This padding should not be necessary for APE tags, because they are stored near the end of the file.
Comment 1 Matej Vadnjal 2006-02-26 13:48:29 UTC
Created attachment 14886 [details]
Proposed patch

Ok, I took a look at the insert() function in file toolkit/tfile.cpp. Turns
out, that ti doesn't actually check if the end of file was reached on first
read (before the "while(bytesRead != 0)" loop, but just assumes that bytesRead
equals bufferLength.

This patch eliminates this bug. And hopefully doesn't introduce new ones.
Comment 2 Lukáš Lalinský 2006-04-20 01:20:58 UTC
Same problem with MPC files (http://bugs.musicbrainz.org/ticket/1304). The patch from #1 fixes it.

Any change this will get into svn soon?

Comment 3 Scott Wheeler 2006-04-20 01:26:53 UTC
Well, for some definition of "soon".  I usually work on projects in batch, especially TagLib.  Usually when there are certain level of bugs I go through and fix almost all of them and before a release I check in fixes for almost all of them.  So, a more accurate answer is that they'll definitely be looked at before the next release.
Comment 4 Scott Wheeler 2007-07-18 18:41:14 UTC
SVN commit 689596 by wheeler:

Make sure that we only write the number of bytes that we read.

BUG:122698


 M  +8 -3      tfile.cpp  


--- trunk/kdesupport/taglib/taglib/toolkit/tfile.cpp #689595:689596
@@ -325,6 +325,7 @@
   // that aren't yet in memory, so this is necessary.
 
   ulong bufferLength = bufferSize();
+
   while(data.size() - replace > bufferLength)
     bufferLength += bufferSize();
 
@@ -352,10 +353,14 @@
 
   buffer = aboutToOverwrite;
 
+  // In case we've already reached the end of file...
+
+  buffer.resize(bytesRead);
+
   // Ok, here's the main loop.  We want to loop until the read fails, which
   // means that we hit the end of the file.
 
-  while(bytesRead != 0) {
+  while(!buffer.isEmpty()) {
 
     // Seek to the current read position and read the data that we're about
     // to overwrite.  Appropriately increment the readPosition.
@@ -375,8 +380,8 @@
     // writePosition.
 
     seek(writePosition);
-    fwrite(buffer.data(), sizeof(char), bufferLength, d->file);
-    writePosition += bufferLength;
+    fwrite(buffer.data(), sizeof(char), buffer.size(), d->file);
+    writePosition += buffer.size();
 
     // Make the current buffer the data that we read in the beginning.