[xiph-commits] r8044 - trunk/oggdsf/src/lib/core/ogg/libOOOgg

illiminable at motherfish-iii.xiph.org illiminable at motherfish-iii.xiph.org
Sun Oct 17 09:55:53 PDT 2004


Author: illiminable
Date: 2004-10-17 09:55:53 -0700 (Sun, 17 Oct 2004)
New Revision: 8044

Modified:
   trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.cpp
   trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.h
   trunk/oggdsf/src/lib/core/ogg/libOOOgg/StampedOggPacket.cpp
Log:
* StampedOggPacket leak free
* Paginator has potential for minor leak... needs further investigation.

Modified: trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.cpp
===================================================================
--- trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.cpp	2004-10-17 16:23:38 UTC (rev 8043)
+++ trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.cpp	2004-10-17 16:55:53 UTC (rev 8044)
@@ -31,8 +31,8 @@
 
 #include "StdAfx.h"
 #include "oggpaginator.h"
+//LEAK CHECK::: Potential for leak based on mPendingPage. if setSettings is called. and also the last page won't get deleted. 20041018
 
-
 //Checksum tables from libogg
 static unsigned long crc_lookup[256]={
   0x00000000,0x04c11db7,0x09823b6e,0x0d4326d9,
@@ -114,15 +114,19 @@
 	,	mPacketCount(0)
 {
 	debugLog.open("G:\\logs\\paginator.log", ios_base::out);
+
+	mHeaderBuff = new unsigned char[300];		//Deleted in destructor.
 	
 }
 
 OggPaginator::~OggPaginator(void)
 {
+	delete mHeaderBuff;
+	mHeaderBuff = NULL;
 	debugLog.close();
 }
 
-
+//Calling this after you have fed in packets will cause lost data and memory leak (mPending page)
 bool OggPaginator::setParameters(OggPaginatorSettings* inSettings) {
 	delete mSettings;
     mSettings = inSettings;
@@ -207,21 +211,24 @@
 	
 		mPendingPage->header()->setCRCChecksum((unsigned long)0);
 
-		unsigned char* locBuff = new unsigned char[300];
-		mPendingPage->header()->rawData(locBuff, 300);
+		//To save memory allocation overhead... there is now one per class. Created in constructor, destroyed in destructor.
+		//unsigned char* locBuff = new unsigned char[300];		//Deleted this function 
+		mPendingPage->header()->rawData(mHeaderBuff, 300);
 
 		for(unsigned long i = 0; i < mPendingPage->headerSize(); i++) {
 			//Create the index we use for the lookup
-			locTemp = ((locChecksum >> 24) & 0xff) ^ locBuff[i];
+			locTemp = ((locChecksum >> 24) & 0xff) ^ mHeaderBuff[i];
 			//XOR the lookup value with the the current checksum shifted left 8 bits.
 			locChecksum=(locChecksum << 8) ^ crc_lookup[locTemp];
 		}
 
-		delete[] locBuff;
-		locBuff = NULL;
+		//Not required, using a single buffer for the class.
+		//delete[] locBuff;
+		//locBuff = NULL;
 
+		unsigned char* locBuff = NULL;
 		for(unsigned long i = 0; i < mPendingPage->numPackets(); i++) {
-			locBuff = mPendingPage->getPacket(i)->packetData();
+			locBuff = mPendingPage->getPacket(i)->packetData();			//View only don't delete.
 
 			for (unsigned long j = 0; j < mPendingPage->getPacket(i)->packetSize(); j++) {
 				locTemp = ((locChecksum >> 24) & 0xff) ^ locBuff[j];
@@ -256,7 +263,7 @@
 }
 bool OggPaginator::createFreshPage() {
 	debugLog<<"Creating fresh page"<<endl;
-	mPendingPage = new OggPage;
+	mPendingPage = new OggPage;			//Potential for leak here if setsettings called. Otherwise deleted in destructor or dispatched
 	mCurrentPageSize = OggPageHeader::OGG_BASE_HEADER_SIZE;
 	mPendingPageHasData = false;
 	mSegmentTableSize = 0;
@@ -384,7 +391,7 @@
 	//debugLog<<"Add part of packet to page"<<endl;
 
 	//Buffer the amount of the packet we are going to add.
-	unsigned char* locBuff = new unsigned char[inLength];
+	unsigned char* locBuff = new unsigned char[inLength];			//Given to constructor of stampedpacket.
 	memcpy((void*)locBuff, (const void*)(inOggPacket->packetData() + inStartFrom), inLength);
 
 	//unsigned long locBytesOfPacketRemaining = inOggPacket->packetSize() - (((locNumSegsNeeded - (255 - mSegmentTableSize)) * 255);
@@ -402,7 +409,7 @@
 																false,   //Not continuation
 																inOggPacket->startTime(), 
 																inOggPacket->endTime(), 
-																inOggPacket->mStampType);
+																inOggPacket->mStampType);		//Given to page.
 
 	//Add the packet to the page.
 	mPendingPage->addPacket(locPartialPacket);
@@ -472,34 +479,6 @@
 
 
 	
-//	unsigned long locNumSegsNeeded = ((inOggPacket->packetSize()) / 255) + 1;
-//	if (locNumSegsNeeded + mSegmentTableSize > 255) {
-//		//Must split the packet... not enough segments
-//		
-//
-//		
-//		deliverCurrentPage();
-//		createFreshPage();
-//		addPartOfPacketToPage(inOggPacket, 0, locRemainingPacketStartsAt);
-//		
-//
-//
-//	} else {
-//		//Just add the packet
-//		mPendingPage->addPacket(inOggPacket->clone());
-//		for (int i = 0; i < locNumSegsNeeded - 1; i++) {
-//			mSegmentTable[mSegmentTableSize] = 255;
-//			mSegmentTableSize++;
-//		}
-//		mSegmentTableSize++;
-//		mSegmentTable[mSegmentTableSize] = (inOggPacket->packetSize() % 255);
-//		mCurrentPageSize += (locNumSegsNeeded + inOggPacket->packetSize());
-//		OggInt64* locGranulePos = new OggInt64;
-//		locGranulePos->setValue(inOggPacket->endTime());
-//		mPendingPage->header()->setGranulePos(locGranulePos);
-//	}
-//	
-//}
 
 
 bool OggPaginator::setPageCallback(IOggCallback* inPageCallback) {

Modified: trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.h
===================================================================
--- trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.h	2004-10-17 16:23:38 UTC (rev 8043)
+++ trunk/oggdsf/src/lib/core/ogg/libOOOgg/OggPaginator.h	2004-10-17 16:55:53 UTC (rev 8044)
@@ -47,13 +47,13 @@
 {
 public:
 	OggPaginator(void);
-	~OggPaginator(void);
+	virtual ~OggPaginator(void);
 
 	bool setParameters(OggPaginatorSettings* inSettings);
 	
+	//IStampedOggPacketSink
+	virtual bool acceptStampedOggPacket(StampedOggPacket* inOggPacket);
 
-	bool acceptStampedOggPacket(StampedOggPacket* inOggPacket);
-
 	bool setPageCallback(IOggCallback* inPageCallback);
 	bool finishStream();
 
@@ -83,6 +83,10 @@
 	OggPaginatorSettings* mSettings;
 	OggPage* mPendingPage;
 
+	unsigned char* mHeaderBuff;
 	fstream debugLog;
-	
+
+private:
+	OggPaginator& operator=(const OggPaginator& other);  /* Don't assign me */
+	OggPaginator(const OggPaginator& other); /* Don't copy me */
 };

Modified: trunk/oggdsf/src/lib/core/ogg/libOOOgg/StampedOggPacket.cpp
===================================================================
--- trunk/oggdsf/src/lib/core/ogg/libOOOgg/StampedOggPacket.cpp	2004-10-17 16:23:38 UTC (rev 8043)
+++ trunk/oggdsf/src/lib/core/ogg/libOOOgg/StampedOggPacket.cpp	2004-10-17 16:55:53 UTC (rev 8044)
@@ -59,7 +59,7 @@
 void StampedOggPacket::merge(StampedOggPacket* inMorePacket) {
 
 	//Make a new buffer the size of both data segs together
-	unsigned char* locBuff = new unsigned char[mPacketSize + inMorePacket->mPacketSize];
+	unsigned char* locBuff = new unsigned char[mPacketSize + inMorePacket->mPacketSize];		//Stored in the member var and deleted by base destructor
 	//Copy this packets data to the start
 	memcpy((void*)locBuff, (const void*)mPacketData, mPacketSize);
 	//Copy the next packets data after it
@@ -91,15 +91,16 @@
 	mIsContinuation = false;
 }
 
+//Returns a packet the caller is responsible for.
 OggPacket* StampedOggPacket::clone() {
 	//Make a new buffer for packet data
-	unsigned char* locBuff = new unsigned char[mPacketSize];
+	unsigned char* locBuff = new unsigned char[mPacketSize];		//Given to constructor of stamped packet... it deletes it.
 
 	//Copy the packet data into the new buffer
 	memcpy((void*)locBuff, (const void*)mPacketData, mPacketSize);
 
 	//Create the new packet
-	StampedOggPacket* retPack = new StampedOggPacket(locBuff, mPacketSize, mIsTruncated, mIsContinuation, mStartTime, mEndTime, mStampType);
+	StampedOggPacket* retPack = new StampedOggPacket(locBuff, mPacketSize, mIsTruncated, mIsContinuation, mStartTime, mEndTime, mStampType);		//Caller takes responsibiility for this.
 	return retPack;
 }
 __int64 StampedOggPacket::startTime() {



More information about the commits mailing list