[Gnash-commit] gnash ChangeLog server/swf/tag_loaders.cpp

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  07/08/09 23:24:44

Modified files:
        .              : ChangeLog 
        server/swf     : tag_loaders.cpp 

Log message:
                * server/swf/tag_loaders.cpp (ADPCMDecoder::adpcm_expand):
                  Rather then throwing a ParseException, just print an error
                  as it's much likely the error is in Gnash parser, rather
                  then in a malformed SWF, and tests show we do a better
                  job like that then by stopping the parser.
                  Also, print an error if the bytesNeeded computation doesn't
                  match the actual number of bytes read.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.3977&r2=1.3978
http://cvs.savannah.gnu.org/viewcvs/gnash/server/swf/tag_loaders.cpp?cvsroot=gnash&r1=1.125&r2=1.126

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.3977
retrieving revision 1.3978
diff -u -b -r1.3977 -r1.3978
--- ChangeLog   9 Aug 2007 21:56:29 -0000       1.3977
+++ ChangeLog   9 Aug 2007 23:24:43 -0000       1.3978
@@ -1,5 +1,15 @@
 2007-08-09 Sandro Santilli <address@hidden>
 
+       * server/swf/tag_loaders.cpp (ADPCMDecoder::adpcm_expand):
+         Rather then throwing a ParseException, just print an error
+         as it's much likely the error is in Gnash parser, rather
+         then in a malformed SWF, and tests show we do a better
+         job like that then by stopping the parser.
+         Also, print an error if the bytesNeeded computation doesn't
+         match the actual number of bytes read.
+
+2007-08-09 Sandro Santilli <address@hidden>
+
        * server/swf/tag_loaders.cpp: move all ADPCM-related code in an
          ADPCMDecoder class.
        * README_CVS: minimum version of automake is 1.9.6. 1.7 and 1.8 have

Index: server/swf/tag_loaders.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/swf/tag_loaders.cpp,v
retrieving revision 1.125
retrieving revision 1.126
diff -u -b -r1.125 -r1.126
--- server/swf/tag_loaders.cpp  9 Aug 2007 21:56:30 -0000       1.125
+++ server/swf/tag_loaders.cpp  9 Aug 2007 23:24:44 -0000       1.126
@@ -17,7 +17,7 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
-/* $Id: tag_loaders.cpp,v 1.125 2007/08/09 21:56:30 strk Exp $ */
+/* $Id: tag_loaders.cpp,v 1.126 2007/08/09 23:24:44 strk Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -226,21 +226,22 @@
                // Total bits needed from stream
                unsigned long bitsNeeded = (compSamples*bitsPerCompSample) + 
(fixedBitsPerBlock*blocksCount);
 
-               // 2 bits have been read already, so the stream position is now 
one byte after the
-               // next 6 bits we're going to read, so we strip those 6 bits 
from the count of bits
-               // we still need
-               bitsNeeded -= (8-2);
-
-               // Now, we convert this number to bytes, requiring one more if 
any 
-               unsigned int excessBits = bitsNeeded%8;
+               // Now, we convert this number to bytes...
                unsigned long bytesNeeded = bitsNeeded/8;
-               if ( excessBits ) ++bytesNeeded;
-
-               //log_debug("adpcm_expand, stereo:%d, sample_count:%u, 
bitsPerSample:%u, "
-               //      "blocksCount:%u, bitsPerBlock:%u, bitsNeeded:%lu, 
excessBits:%u, bytesNeeded:%lu",
-               //      stereo, sample_count, bitsPerCompSample, blocksCount, 
fixedBitsPerBlock, bitsNeeded, excessBits, bytesNeeded);
+               // ... requiring one more if the bits in excess are more then
+               //     the ones still available in last byte read 
+               unsigned int excessBits = bitsNeeded%8;
+               if ( excessBits > 6 ) ++bytesNeeded;
 
-               in->ensureBytes(bytesNeeded);
+               // Take note of the current position to later verify if we got 
the 
+               // number of required bytes right
+               unsigned long prevPosition = in->get_position();
+
+               // We substract 1 byte as the 6 excessive of a byte are already 
in the stream,
+               // and we won't require another one unless more then 6 
excessive bits are needed
+               // WARNING: this is currently disabled due to a bug in this 
function often resulting
+               //          in reads past the end of the stream
+               //in->ensureBytes(bytesNeeded-1);
 
 #endif // GNASH_TRUST_SWF_INPUT
 
@@ -297,6 +298,39 @@
                        }
                }
 
+#ifndef GNASH_TRUST_SWF_INPUT
+               unsigned long curPos = in->get_position();
+               unsigned long bytesRead = curPos - prevPosition;
+               if ( bytesRead != bytesNeeded )
+               {
+                       // This would happen if the computation of bytesNeeded 
doesn't match the current
+                       // implementation.
+                       // NOTE That the current implementation seems pretty 
much bogus as we *often* end
+                       // up reading past the end of the tag. Once we fix the 
decoding we shoudl also fix
+                       // the computation of bytes needed
+                       log_error("admcp_expand: we expected to read %lu bytes, 
but we read %lu instead (%ld error)",
+                               bytesNeeded, bytesRead, bytesNeeded-bytesRead);
+                       // abort();
+               }
+
+               unsigned long endTagPos = in->get_tag_end_position();
+               if ( curPos > endTagPos )
+               {
+                       // This happens when our decoder reads past the end of 
the tag.
+                       // In general, we should aborth parsing of the current 
tag when this happens,
+                       // anyway, it seems that *some* sound can be heard 
nonetheless so we keep going.
+                       log_error("admcp_expand: read past tag boundary: 
current position: %lu, end of tag position: %lu (overflow: %lu bytes)",
+                               curPos, endTagPos, curPos-endTagPos);
+
+#if 0
+                       log_debug("      stereo:%d, sample_count:%u, 
compressedSamples:%d, bitsPerCompSample:%u, "
+                               "blocksCount:%u, bitsPerBlock:%u, 
bitsNeeded:%lu, excessBits:%u, bytesNeeded:%lu, bytesLeft:%lu",
+                               stereo, sample_count, compSamples, 
bitsPerCompSample, blocksCount, fixedBitsPerBlock,
+                               bitsNeeded, excessBits, bytesNeeded, 
in->get_tag_end_position()-in->get_position());
+#endif
+               }
+#endif // GNASH_TRUST_SWF_INPUT
+
        }