[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebob-cvs
Subject:    [Freebob-cvs] SF.net SVN: freebob: [309]
From:       jwoithe () users ! sourceforge ! net
Date:       2006-09-25 0:50:28
Message-ID: E1GReg0-0001JD-Ht () sc8-pr-svn1 ! sourceforge ! net
[Download RAW message or body]

Revision: 309
          http://svn.sourceforge.net/freebob/?rev=309&view=rev
Author:   jwoithe
Date:     2006-09-24 17:50:17 -0700 (Sun, 24 Sep 2006)

Log Message:
-----------
MOTU: Fixed false "missed rx cycle" report following xrun recovery.
Ensure iso rx/tx contexts are deallocated during shutdown/xrun recovery by explicitly \
deleting IsoHandlers in IsoHandlerManager::pruneHandlers().  If they aren't deleted \
here they never get deleted because the reference is lost. IsoHandler destructor \
should only call stop() if the handle is valid. IsoXmitHandler's destructor sets the \
handle NULL to prevent double-free by the inherited IsoHandler destructor. Don't call \
raw1394_iso_shutdown() from our code.  libraw1394 1.2.1 has a bug whereby \
raw1394_new_handle() fails to initialise the iso_packet_infos field.  The bug hits us \
particularly in IsoRecvHandler::prepare().  It's also not really necessary to call \
raw1394_iso_shutdown() since raw1394_destroy_handle() will do any cleanups we happen \
                to need.
MOTU: the receive stream no longer falsely complains of buffer problems during device \
                shutdown.
MOTU: fixed a false "missed cycle" detection immediately after the stream was \
enabled.

Modified Paths:
--------------
    branches/libfreebob-2.0/src/libstreaming/IsoHandler.cpp
    branches/libfreebob-2.0/src/libstreaming/IsoHandler.h
    branches/libfreebob-2.0/src/libstreaming/IsoHandlerManager.cpp
    branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.cpp
    branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.h
    branches/libfreebob-2.0/src/libstreaming/StreamProcessorManager.cpp

Modified: branches/libfreebob-2.0/src/libstreaming/IsoHandler.cpp
===================================================================
--- branches/libfreebob-2.0/src/libstreaming/IsoHandler.cpp	2006-09-21 23:18:50 UTC \
                (rev 308)
+++ branches/libfreebob-2.0/src/libstreaming/IsoHandler.cpp	2006-09-25 00:50:17 UTC \
(rev 309) @@ -80,8 +80,10 @@
 /* Base class implementation */
 
 IsoHandler::~IsoHandler() {
-    stop();
-    if(m_handle) raw1394_destroy_handle(m_handle);
+    if(m_handle) {
+        stop();
+        raw1394_destroy_handle(m_handle);
+    }
     if(m_handle_util) raw1394_destroy_handle(m_handle_util);
     
 }
@@ -227,9 +229,13 @@
 }
 IsoRecvHandler::~IsoRecvHandler()
 {
-	raw1394_iso_shutdown(m_handle);
+// Don't call until libraw1394's raw1394_new_handle() function has been
+// fixed to correctly initialise the iso_packet_infos field.  Bug is
+// confirmed present in libraw1394 1.2.1.  In any case,
+// raw1394_destroy_handle() will do any iso system shutdown required.
+//	raw1394_iso_shutdown(m_handle);
 	raw1394_destroy_handle(m_handle);
-
+	m_handle = NULL;
 }
 
 bool
@@ -262,7 +268,10 @@
 
 bool IsoRecvHandler::prepare()
 {
-	raw1394_iso_shutdown(m_handle);
+// Don't call until libraw1394's raw1394_new_handle() function has been
+// fixed to correctly initialise the iso_packet_infos field.  Bug is
+// confirmed present in libraw1394 1.2.1.
+//	raw1394_iso_shutdown(m_handle);
 	
 	debugOutput( DEBUG_LEVEL_VERBOSE, "Preparing iso receive handler (%p)\n",this);
 	debugOutput( DEBUG_LEVEL_VERBOSE, " Buffers         : %d \n",m_buf_packets);
@@ -329,9 +338,13 @@
 
 IsoXmitHandler::~IsoXmitHandler()
 {
-	debugOutput( DEBUG_LEVEL_VERBOSE, "enter...\n");
-	raw1394_iso_shutdown(m_handle);
+// Don't call until libraw1394's raw1394_new_handle() function has been
+// fixed to correctly initialise the iso_packet_infos field.  Bug is
+// confirmed present in libraw1394 1.2.1.  In any case,
+// raw1394_destroy_handle() will do any iso system shutdown required.
+//	raw1394_iso_shutdown(m_handle);
 	raw1394_destroy_handle(m_handle);
+	m_handle = NULL;
 }
 
 bool
@@ -424,6 +437,11 @@
 }
 IsoRecvHandler::~IsoRecvHandler()
 {
+// Don't call until libraw1394's raw1394_new_handle() function has been
+// fixed to correctly initialise the iso_packet_infos field.  Bug is
+// confirmed present in libraw1394 1.2.1.  In any case,
+// raw1394_destroy_handle() (in the base class destructor) will do any iso
+// system shutdown required.
 	raw1394_iso_shutdown(m_handle);
 
 }

Modified: branches/libfreebob-2.0/src/libstreaming/IsoHandler.h
===================================================================
--- branches/libfreebob-2.0/src/libstreaming/IsoHandler.h	2006-09-21 23:18:50 UTC \
                (rev 308)
+++ branches/libfreebob-2.0/src/libstreaming/IsoHandler.h	2006-09-25 00:50:17 UTC \
(rev 309) @@ -111,7 +111,7 @@
 		virtual bool prepare() = 0;
 		
 		unsigned int getCycleCounter();
-    
+
 	protected:
 	    raw1394handle_t m_handle;
         raw1394handle_t m_handle_util;

Modified: branches/libfreebob-2.0/src/libstreaming/IsoHandlerManager.cpp
===================================================================
--- branches/libfreebob-2.0/src/libstreaming/IsoHandlerManager.cpp	2006-09-21 \
                23:18:50 UTC (rev 308)
+++ branches/libfreebob-2.0/src/libstreaming/IsoHandlerManager.cpp	2006-09-25 \
00:50:17 UTC (rev 309) @@ -172,7 +172,6 @@
 			return rebuildFdMap();
 		}
 	}
-
 	debugFatal("Could not find handler (%p)\n", handler);
 	
 	return false; //not found
@@ -469,6 +468,16 @@
     {
 		unregisterHandler(*it);
 		debugOutput( DEBUG_LEVEL_VERBOSE, " deleting handler (%p)\n",*it);
+
+		// Now the handler's been unregistered it won't be reused
+		// again.  Therefore it really needs to be formally deleted
+		// to free up the raw1394 handle.  Otherwise things fall
+		// apart after several xrun recoveries as the system runs
+		// out of resources to support all the disused but still
+		// allocated raw1394 handles.  At least this is the current
+		// theory as to why we end up with "memory allocation"
+		// failures after several Xrun recoveries.
+		delete *it;
     }
 
 }

Modified: branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.cpp
===================================================================
--- branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.cpp	2006-09-21 \
                23:18:50 UTC (rev 308)
+++ branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.cpp	2006-09-25 \
00:50:17 UTC (rev 309) @@ -112,6 +112,12 @@
 		m_cycle_ofs = 0.0;
 	}
 
+	// Similarly, initialise the "next cycle".  This can be done
+	// whenever iso data is seen - it doesn't have to wait until
+	// the stream is initialised.
+	if (m_next_cycle < 0)
+		m_next_cycle = cycle;
+
 	// Do housekeeping expected for all packets sent to the MOTU, even
 	// for packets containing no audio data.
 	*sy = 0x00;
@@ -168,8 +174,7 @@
 		m_next_cycle = cycle;
 	}
 
-
-	if (!m_disabled) {
+	if  (!m_disabled) {
 		if (++m_next_cycle >= 8000)
 			m_next_cycle -= 8000;
 	} else
@@ -870,7 +875,7 @@
 MotuReceiveStreamProcessor::MotuReceiveStreamProcessor(int port, int framerate, 
 	unsigned int event_size)
     : ReceiveStreamProcessor(port, framerate), m_event_size(event_size),
-	m_last_cycle_ofs(-1), m_next_cycle(-1) {
+	m_last_cycle_ofs(-1), m_next_cycle(-1), m_closedown_active(0) {
 
 	// Set up the Delay-locked-loop to track audio frequency relative
 	// to the cycle timer.  The seed value is the "ideal" value.
@@ -905,11 +910,11 @@
 
     // Detect missed receive cycles
     // FIXME: it would be nice to advance the rx buffer by the amount of
-    // frames missed.  However, since the MOTU transmits more frames 
-    // per cycle than the average and "catches up" with period emty
-    // cycles it's not trivial to work out precisely how many frames
-    // were missed.  Ultimately we need to do so if sync is to be 
-    // maintained across a transient receive failure.
+    // frames missed.  However, since the MOTU transmits more frames per
+    // cycle than the average and "catches up" with periodic empty cycles
+    // it's not trivial to work out precisely how many frames were missed. 
+    // Ultimately I think we need to do so if sync is to be maintained
+    // across a transient receive failure.
     if (m_next_cycle < 0)
 	m_next_cycle = cycle;
     if ((signed)cycle != m_next_cycle) {
@@ -918,8 +923,11 @@
 	m_next_cycle = cycle;
 	have_lost_cycles = 1;
     }
-    if (++m_next_cycle >= 8000)
-	m_next_cycle -= 8000;
+    if (!m_disabled) {
+        if (++m_next_cycle >= 8000)
+	    m_next_cycle -= 8000;
+    } else
+        m_next_cycle = -1;
 
     // If the packet length is 8 bytes (ie: just a CIP-like header) there is
     // no isodata.
@@ -982,11 +990,20 @@
 		m_last_cycle_ofs = sph_ofs;
 	}
 
-	// Don't process the stream when it is not enabled.
+	// Don't process the stream when it is not enabled
 	if (m_disabled) {
 		return RAW1394_ISO_OK;
 	}
         
+	// If closedown is active we also just throw data way, but in this case
+	// we keep the frame counter going to prevent a false xrun detection
+	if (m_closedown_active) {
+		incrementFrameCounter(n_events);
+		if (m_framecounter > (signed int)m_period)
+			return RAW1394_ISO_DEFER;
+		return RAW1394_ISO_OK;
+	}
+
 	debugOutput( DEBUG_LEVEL_VERY_VERBOSE, "put packet...\n");
 
         // Add the data payload (events) to the ringbuffer.  We'll just copy
@@ -1064,6 +1081,8 @@
 		debugFatal("Could not do base class reset\n");
 		return false;
 	}
+
+	m_next_cycle = -1;
 	
 	return true;
 }
@@ -1184,6 +1203,11 @@
 	// This is period_size*m_event_size bytes.
 	unsigned int bytes2read = m_period * m_event_size;
 
+	// If closedown is in progress just pretend that data's been transferred
+	// to prevent false underrun detections on the event buffer.
+	if (m_closedown_active)
+		return true;
+
 	/* Read events2read bytes from the ringbuffer.
 	*  First see if it can be done in one read.  If so, ok.
 	*  Otherwise read up to a multiple of events directly from the buffer
@@ -1405,5 +1429,17 @@
 //
 	return m_event_size;
 }
+
+bool MotuReceiveStreamProcessor::preparedForStop() {
+
+	// A MOTU receive stream can stop at any time.  However, signify
+	// that stopping is in progress because other streams (notably the
+	// transmit stream) may keep going for some time and cause an
+	// overflow in the receive buffers.  If a closedown is in progress
+	// the receive handler simply throws all incoming data away so
+	// no buffer overflow can occur.
+	m_closedown_active = 1;
+	return true;
+}
                 
 } // end of namespace FreebobStreaming

Modified: branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.h
===================================================================
--- branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.h	2006-09-21 \
                23:18:50 UTC (rev 308)
+++ branches/libfreebob-2.0/src/libstreaming/MotuStreamProcessor.h	2006-09-25 \
00:50:17 UTC (rev 309) @@ -163,6 +163,8 @@
 	signed int setEventSize(unsigned int size);
 	unsigned int getEventSize(void);
 
+	virtual bool preparedForStop();
+
 protected:
 
 	int receiveBlock(char *data, unsigned int nevents, unsigned int offset);
@@ -186,6 +188,10 @@
 	signed int m_last_cycle_ofs;
 	signed int m_next_cycle;
 
+	// Signifies a closedown is in progress, in which case incoming data 
+        // is junked.
+        signed int m_closedown_active;
+
     DECLARE_DEBUG_MODULE;
 
 };

Modified: branches/libfreebob-2.0/src/libstreaming/StreamProcessorManager.cpp
===================================================================
--- branches/libfreebob-2.0/src/libstreaming/StreamProcessorManager.cpp	2006-09-21 \
                23:18:50 UTC (rev 308)
+++ branches/libfreebob-2.0/src/libstreaming/StreamProcessorManager.cpp	2006-09-25 \
00:50:17 UTC (rev 309) @@ -356,7 +356,7 @@
 	m_streamingThread->Start();
 	
 	debugOutput( DEBUG_LEVEL_VERBOSE, "Waiting for all StreamProcessors to start \
                running...\n");
-	// we have to wait untill all streamprocessors indicate that they are running
+	// we have to wait until all streamprocessors indicate that they are running
 	// i.e. that there is actually some data stream flowing
 	int wait_cycles=2000; // two seconds
 	bool notRunning=true;
@@ -515,7 +515,6 @@
 				return false;
 			}
 			
-			
 		}
 
 	debugOutput( DEBUG_LEVEL_VERBOSE, " Transmit processors...\n");
@@ -528,7 +527,6 @@
 			}
 			
 		}
-    
 	
 	return true;
 	


This was sent by the SourceForge.net collaborative development platform, the world's \
largest Open Source development site.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Freebob-cvs mailing list
Freebob-cvs@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freebob-cvs


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic