[PATCH] Attempt to fix nextFrameOffset and previousFrameOffset for synch spanning buffer read boundary

Xavier Duret xaviour.maillists at gmail.com
Wed Jan 24 19:24:02 CET 2007


The subject of the mail says it all. This is an attempt because while
this patch does not cause regression in my test suite, it is also
likely that there are no files in it that match the boundary
condition. This being said, this patch is fairly reviewer friendly.

diff -ruN taglib.old/taglib/mpeg/mpegfile.cpp taglib/taglib/mpeg/mpegfile.cpp
--- taglib.old/taglib/mpeg/mpegfile.cpp   2007-01-24 18:37:09.000000000 +0100
+++ taglib/taglib/mpeg/mpegfile.cpp       2007-01-24 18:39:15.000000000 +0100
@@ -445,18 +445,22 @@

 long MPEG::File::nextFrameOffset(long position)
 {
-  // TODO: This will miss syncs spanning buffer read boundaries.
-
+  uchar lastByte = 0;
   ByteVector buffer(bufferSize(),0);

   while(buffer.size() > 0) {
     seek(position);
     buffer = readBlock(bufferSize());

+    if(lastByte == 0xff && secondSynchByte(buffer[0]))
+      return position - 1;
+
     for(uint i = 0; i < buffer.size() - 1; i++) {
       if(uchar(buffer[i]) == 0xff && secondSynchByte(buffer[i + 1]))
        return position + i;
     }
+
+    lastByte = buffer[buffer.size() - 1];
     position += bufferSize();
   }

@@ -465,7 +469,7 @@

 long MPEG::File::previousFrameOffset(long position)
 {
-  // TODO: This will miss syncs spanning buffer read boundaries.
+  uchar lastByte = 0;

   while(int(position - bufferSize()) > int(bufferSize())) {
     position -= bufferSize();
@@ -478,10 +482,15 @@
     if(buffer.size() < 4)
       return -1;

+    if(uchar(buffer[buffer.size() - 1])== 0xff && secondSynchByte(lastByte))
+      return position + buffer.size() - 1;
+
     for(int i = buffer.size() - 2; i >= 0; i--) {
       if(uchar(buffer[i]) == 0xff && secondSynchByte(buffer[i + 1]))
         return position + i;
     }
+
+    lastByte = buffer[0];
   }

   return -1;


More information about the taglib-devel mailing list