[Gnash-commit] gnash ChangeLog libbase/FLVParser.cpp libbase/F...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  08/05/10 11:05:28

Modified files:
        .              : ChangeLog 
        libbase        : FLVParser.cpp FLVParser.h utility.h 

Log message:
                * libbase/utility.h: add smallestMultipleContaining
                  utility function.
                * libbase/FLVParser.{cpp,h}: always allocate a multiple
                  of 64 (READ_CHUNK) greater or equal the actual frame
                  size augmented by 64 bytes (PADDING_BYTES): the multiple
                  thing was suggested by bjacques, the PADDING_BYTES I found
                  by myself (for h264 parser in ffmpeg...). Always check
                  return from tu_file::read_bytes to catch premature end
                  of input. Merge duplicated code into makeAudioFrame and
                  makeVideoFrame static functions.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6576&r2=1.6577
http://cvs.savannah.gnu.org/viewcvs/gnash/libbase/FLVParser.cpp?cvsroot=gnash&r1=1.30&r2=1.31
http://cvs.savannah.gnu.org/viewcvs/gnash/libbase/FLVParser.h?cvsroot=gnash&r1=1.25&r2=1.26
http://cvs.savannah.gnu.org/viewcvs/gnash/libbase/utility.h?cvsroot=gnash&r1=1.49&r2=1.50

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6576
retrieving revision 1.6577
diff -u -b -r1.6576 -r1.6577
--- ChangeLog   9 May 2008 19:34:11 -0000       1.6576
+++ ChangeLog   10 May 2008 11:05:26 -0000      1.6577
@@ -1,3 +1,16 @@
+2008-05-10 Sandro Santilli <address@hidden>
+
+       * libbase/utility.h: add smallestMultipleContaining
+         utility function.
+       * libbase/FLVParser.{cpp,h}: always allocate a multiple
+         of 64 (READ_CHUNK) greater or equal the actual frame
+         size augmented by 64 bytes (PADDING_BYTES): the multiple
+         thing was suggested by bjacques, the PADDING_BYTES I found
+         by myself (for h264 parser in ffmpeg...). Always check
+         return from tu_file::read_bytes to catch premature end
+         of input. Merge duplicated code into makeAudioFrame and
+         makeVideoFrame static functions.
+
 2008-05-09 Benjamin Wolsey <address@hidden>
 
        * gui/gnash.cpp: add comments for options handled elsewhere. What is

Index: libbase/FLVParser.cpp
===================================================================
RCS file: /sources/gnash/gnash/libbase/FLVParser.cpp,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -b -r1.30 -r1.31
--- libbase/FLVParser.cpp       9 May 2008 15:33:45 -0000       1.30
+++ libbase/FLVParser.cpp       10 May 2008 11:05:27 -0000      1.31
@@ -23,16 +23,76 @@
 #include "FLVParser.h"
 #include "amf.h"
 #include "log.h"
+#include "utility.h"
 
 using namespace std;
 
 #define PADDING_BYTES 64
+#define READ_CHUNKS 64
 
 // Define the following macro the have seek() operations printed
 //#define GNASH_DEBUG_SEEK 1
 
 namespace gnash {
 
+static std::auto_ptr<FLVFrame>
+makeVideoFrame(tu_file& in, const FLVVideoFrame& frameInfo)
+{
+       std::auto_ptr<FLVFrame> frame ( new FLVFrame );
+
+       frame->dataSize = frameInfo.dataSize;
+       frame->timestamp = frameInfo.timestamp;
+       frame->tag = 9;
+
+       if ( in.set_position(frameInfo.dataPosition) )
+       {
+               log_error(_("Failed seeking to videoframe in FLV input"));
+               frame.reset();
+               return frame;
+       }
+
+       unsigned long int dataSize = frameInfo.dataSize;
+       unsigned long int chunkSize = smallestMultipleContaining(READ_CHUNKS, 
dataSize+PADDING_BYTES);
+
+       frame->data = new boost::uint8_t[chunkSize];
+       size_t bytesread = in.read_bytes(frame->data, dataSize);
+
+       unsigned long int padding = chunkSize-dataSize;
+       assert(padding);
+       memset(frame->data + bytesread, 0, padding);
+
+       return frame;
+}
+
+static std::auto_ptr<FLVFrame>
+makeAudioFrame(tu_file& in, const FLVAudioFrame& frameInfo)
+{
+       std::auto_ptr<FLVFrame> frame ( new FLVFrame );
+       frame->dataSize = frameInfo.dataSize; 
+       frame->timestamp = frameInfo.timestamp;
+
+       if ( in.set_position(frameInfo.dataPosition) )
+       {
+               log_error(_("Failed seeking to audioframe in FLV input"));
+               frame.reset();
+               return frame;
+       }
+
+       unsigned long int dataSize = frameInfo.dataSize;
+       unsigned long int chunkSize = smallestMultipleContaining(READ_CHUNKS, 
dataSize+PADDING_BYTES);
+
+       frame->data = new boost::uint8_t[chunkSize];
+       size_t bytesread = in.read_bytes(frame->data, dataSize);
+
+       unsigned long int padding = chunkSize-dataSize;
+       assert(padding);
+       memset(frame->data + bytesread, 0, padding);
+
+       frame->tag = 8;
+
+       return frame;
+}
+
 FLVParser::FLVParser(tu_file& lt)
        :
        _lt(lt),
@@ -162,32 +222,30 @@
 
        if (useAudio) {
 
-               FLVFrame* frame = new FLVFrame;
-               frame->dataSize = _audioFrames[_nextAudioFrame]->dataSize;
-               frame->timestamp = _audioFrames[_nextAudioFrame]->timestamp;
-
-               _lt.set_position(_audioFrames[_nextAudioFrame]->dataPosition); 
// TODO: check return code...
-               frame->data = new boost::uint8_t[frame->dataSize + 
PADDING_BYTES];
-               size_t bytesread = _lt.read_bytes(frame->data, frame->dataSize);
-               memset(frame->data + bytesread, 0, PADDING_BYTES);
+               FLVAudioFrame* frameInfo = _audioFrames[_nextAudioFrame];
+
+               std::auto_ptr<FLVFrame> frame = makeAudioFrame(_lt, *frameInfo);
+               if ( ! frame.get() )
+               {
+                       log_error("Could not make audio frame %d", 
_nextAudioFrame);
+                       return 0;
+               }
 
-               frame->tag = 8;
                _nextAudioFrame++;
-               return frame;
+               return frame.release(); // TODO: return by auto_ptr
 
        } else {
-               FLVFrame* frame = new FLVFrame;
-               frame->dataSize = _videoFrames[_nextVideoFrame]->dataSize;
-               frame->timestamp = _videoFrames[_nextVideoFrame]->timestamp;
-
-               _lt.set_position(_videoFrames[_nextVideoFrame]->dataPosition); 
// TODO: check return code
-               frame->data = new boost::uint8_t[frame->dataSize + 
PADDING_BYTES];
-               size_t bytesread  = _lt.read_bytes(frame->data, 
frame->dataSize);
-               memset(frame->data + bytesread, 0, PADDING_BYTES);
 
-               frame->tag = 9;
+               FLVVideoFrame* frameInfo = _videoFrames[_nextVideoFrame];
+               std::auto_ptr<FLVFrame> frame = makeVideoFrame(_lt, *frameInfo);
+               if ( ! frame.get() )
+               {
+                       log_error("Could not make video frame %d", 
_nextVideoFrame);
+                       return 0;
+               }
+
                _nextVideoFrame++;
-               return frame;
+               return frame.release(); // TODO: return by auto_ptr
        }
 
 
@@ -208,20 +266,16 @@
        // If the needed frame can't be parsed (EOF reached) return NULL
        if (_audioFrames.size() <= _nextAudioFrame || _audioFrames.size() == 0) 
return NULL;
 
-       FLVFrame* frame = new FLVFrame;
-       frame->dataSize = _audioFrames[_nextAudioFrame]->dataSize;
-       frame->timestamp = _audioFrames[_nextAudioFrame]->timestamp;
-       frame->tag = 8;
-
-       _lt.set_position(_audioFrames[_nextAudioFrame]->dataPosition); // TODO: 
check return code
-       frame->data = new 
boost::uint8_t[_audioFrames[_nextAudioFrame]->dataSize +
-                                 PADDING_BYTES];
-       size_t bytesread = _lt.read_bytes(frame->data, 
-                               _audioFrames[_nextAudioFrame]->dataSize);
-       memset(frame->data + bytesread, 0, PADDING_BYTES);
+       FLVAudioFrame* frameInfo = _audioFrames[_nextAudioFrame];
+       std::auto_ptr<FLVFrame> frame = makeAudioFrame(_lt, *frameInfo);
+       if ( ! frame.get() )
+       {
+               log_error("Could not make audio frame %d", _nextAudioFrame);
+               return 0;
+       }
 
        _nextAudioFrame++;
-       return frame;
+       return frame.release(); // TODO: return by auto_ptr
 
 }
 
@@ -249,21 +303,17 @@
                return NULL;
        }
 
-
-       FLVFrame* frame = new FLVFrame;
-       frame->dataSize = _videoFrames[_nextVideoFrame]->dataSize;
-       frame->timestamp = _videoFrames[_nextVideoFrame]->timestamp;
-       frame->tag = 9;
-
-       _lt.set_position(_videoFrames[_nextVideoFrame]->dataPosition); // TODO: 
check return code
-       frame->data = new 
boost::uint8_t[_videoFrames[_nextVideoFrame]->dataSize + 
-                                 PADDING_BYTES];
-       size_t bytesread = _lt.read_bytes(frame->data, 
-                               _videoFrames[_nextVideoFrame]->dataSize);
-       memset(frame->data + bytesread, 0, PADDING_BYTES);
+       // TODO: let a function do this
+       FLVVideoFrame* frameInfo = _videoFrames[_nextVideoFrame];
+       std::auto_ptr<FLVFrame> frame = makeVideoFrame(_lt, *frameInfo);
+       if ( ! frame.get() )
+       {
+               log_error("Could not make video frame %d", _nextVideoFrame);
+               return 0;
+       }
 
        _nextVideoFrame++;
-       return frame;
+       return frame.release(); // TODO: return by auto_ptr
 }
 
 
@@ -528,7 +578,12 @@
 
        // Read the tag info
        boost::uint8_t tag[12];
-       _lt.read_bytes(tag, 12);
+       int actuallyRead = _lt.read_bytes(tag, 12);
+       if ( actuallyRead < 12 )
+       {
+               log_error("FLVParser::parseNextFrame: can't read tag info 
(needed 12 bytes, only got %d)", actuallyRead);
+               return false;
+       }
 
        // Extract length and timestamp
        boost::uint32_t bodyLength = getUInt24(&tag[1]);
@@ -590,7 +645,12 @@
                                        return false;
                                }
                                boost::uint8_t videohead[12];
-                               _lt.read_bytes(videohead, 12); // TODO: use 
return code
+                               int actuallyRead = _lt.read_bytes(videohead, 
12);
+                               if ( actuallyRead < 12 )
+                               {
+               log_error("FLVParser::parseNextFrame: can't read H263 video 
header (needed 12 bytes, only got %d)", actuallyRead);
+               return false;
+                               }
 
                                bool sizebit1 = (videohead[3] & 0x02);
                                bool sizebit2 = (videohead[3] & 0x01);
@@ -633,6 +693,7 @@
                }
 
        } else if (tag[0] == META_TAG) {
+               LOG_ONCE( log_unimpl("FLV MetaTag parser") );
                // Extract information from the meta tag
                /*_lt.seek(_lastParsedPosition+16);
                char* metaTag = new char[bodyLength];
@@ -691,3 +752,4 @@
 } // end of gnash namespace
 
 #undef PADDING_BYTES
+#undef READ_CHUNKS 

Index: libbase/FLVParser.h
===================================================================
RCS file: /sources/gnash/gnash/libbase/FLVParser.h,v
retrieving revision 1.25
retrieving revision 1.26
diff -u -b -r1.25 -r1.26
--- libbase/FLVParser.h 9 May 2008 15:33:45 -0000       1.25
+++ libbase/FLVParser.h 10 May 2008 11:05:28 -0000      1.26
@@ -91,14 +91,21 @@
 };
 
 
+/// Information about an FLV Audio Frame
 class FLVVideoFrame
 {
 public:
+
+       /// Type of this frame (should likely be videoFrameType)
        boost::uint16_t frameType;
+
+       /// Size of the frame in bytes
        boost::uint32_t dataSize;
+
+       /// Start of frame data in stream
        boost::uint64_t dataPosition;
 
-       /// in milliseconds 
+       /// Timestamp in milliseconds 
        boost::uint32_t timestamp;
 
        /// Return true if this video frame is a key frame
@@ -109,13 +116,17 @@
 
 };
 
+/// Information about an FLV Audio Frame
 class FLVAudioFrame
 {
 public:
+       /// Size of the frame in bytes
        boost::uint32_t dataSize;
+
+       /// Start of frame data in stream
        boost::uint64_t dataPosition;
 
-       /// in milliseconds 
+       /// Timestamp in milliseconds 
        boost::uint32_t timestamp;
 
 };
@@ -307,11 +318,17 @@
        /// The interface to the file, externally owned
        tu_file& _lt;
 
+       // NOTE: FLVVideoFrame is a relatively small structure,
+       //       chances are keeping by value here would reduce
+       //       memory fragmentation with no big cost
        typedef std::vector<FLVVideoFrame*> VideoFrames;
 
        /// list of videoframes, does no contain the frame data.
        VideoFrames _videoFrames;
 
+       // NOTE: FLVAudioFrame is a relatively small structure,
+       //       chances are keeping by value here would reduce
+       //       memory fragmentation with no big cost
        typedef std::vector<FLVAudioFrame*> AudioFrames;
 
        /// list of audioframes, does no contain the frame data.

Index: libbase/utility.h
===================================================================
RCS file: /sources/gnash/gnash/libbase/utility.h,v
retrieving revision 1.49
retrieving revision 1.50
diff -u -b -r1.49 -r1.50
--- libbase/utility.h   29 Apr 2008 17:35:42 -0000      1.49
+++ libbase/utility.h   10 May 2008 11:05:28 -0000      1.50
@@ -132,6 +132,18 @@
 inline double trunc(double x) { return (x < 0 ?  -(std::floor(-x)) : 
std::floor(x)); }
 #endif
 
+
+/// \brief
+/// Return the smallest multiple of given base greater or equal
+/// given limit
+inline unsigned int
+smallestMultipleContaining(unsigned int base, unsigned int x)
+{
+        int f=x/base;
+        if ( x%base ) f++;
+        return base*f;
+}
+
 // Handy macro to quiet compiler warnings about unused parameters/variables.
 #define UNUSED(x) (x) = (x)