Removed time sourced from reference counting.

Releasing nodes works now, except if they have been instanciated globally.


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@2920 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/media/MediaMisc.h

@@ -15,6 +15,10 @@
 
 #define NODE_UNREGISTERED_ID	-2
 
+#define SHADOW_TIMESOURCE_CONTROL_PORT	-333
+
+#define NODE_KIND_NO_REFCOUNTING		0x80000000
+
 #define ROUND_UP_TO_PAGE(size)			(((size) + B_PAGE_SIZE - 1) & ~(B_PAGE_SIZE - 1))
 
 namespace BPrivate { namespace media {

src/kits/media/MediaNode.cpp

@@ -195,10 +195,17 @@ BTimeSource *
 BMediaNode::TimeSource() const
 {
 	CALLED();
+	
+	// return the currently assigned time source
 	if (fTimeSource != 0)
 		return fTimeSource;
 	
-	BMediaNode *self = const_cast<BMediaNode *>(this);
+	// If the node hasn't been assigned a time source
+	// so far, we assign the system time source. This
+	// can't be done in the BMediaNode constructor, since
+	// a BTimeSource is also a BMediaNode that would be
+	// a infinite loop... loop... loop... loop...
+	
 	BMediaRoster *roster = BMediaRoster::Roster();
 	status_t rv;
 	media_node clone;
@@ -207,6 +214,8 @@ BMediaNode::TimeSource() const
 		FATAL("BMediaNode::TimeSource: Error, GetSystemTimeSource failed\n");
 		return NULL;
 	}
+
+	BMediaNode *self = const_cast<BMediaNode *>(this);
 	self->fTimeSource = roster->MakeTimeSourceFor(clone);
 	ASSERT(fTimeSource == self->fTimeSource);
 	if (fTimeSource == 0) {
@@ -214,10 +223,11 @@ BMediaNode::TimeSource() const
 	} else {
 		fTimeSource->AddMe(self);
 	}
-	rv = roster->ReleaseNode(clone);
+
-	if (rv != B_OK) {
+//	rv = roster->ReleaseNode(clone);
-		FATAL("BMediaNode::TimeSource: Error, ReleaseNode failed\n");
+//	if (rv != B_OK) {
-	}
+//		FATAL("BMediaNode::TimeSource: Error, ReleaseNode failed\n");
+//	}
 
 	return fTimeSource;
 }
@@ -466,13 +476,18 @@ BMediaNode::SetTimeSource(BTimeSource *time_source)
 	// this is a hook function, and 
 	// may be overriden by derived classes.
 
+	if (time_source == NULL || time_source == fTimeSource)
+		return;
+	
+	// we just trip into debugger, code that tries to do this is broken.
+	debugger("BMediaNode::SetTimeSource() can't be used to set a timesource, use BMediaRoster::SetTimeSourceFor()!\n");
+
+	/*
+	FATAL("BMediaNode::SetTimeSource used to set a time source for this node\n");
 
 	// the functionality here is only to
 	// support those people that don't
 	// use the roster to set a time source
-	if (time_source == NULL || time_source == fTimeSource)
-		return;
-	FATAL("BMediaNode::SetTimeSource used to set a time source for this node\n");
 
 	// some stupid code to do a stupid thing that should not be done
 	BMediaNode *newnode = time_source->Acquire();
@@ -486,9 +501,12 @@ BMediaNode::SetTimeSource(BTimeSource *time_source)
 		fTimeSource->RemoveMe(this);
 		fTimeSource->Release();
 		newsource->AddMe(this);
+		printf("#### BMediaNode::SetTimeSource: node %ld has been assigned  time source %ld\n", ID(), time_source->ID());
+			
 	}
 	fTimeSource = newsource;
 	//BMediaRoster::Roster()->StartTimeSource(fTimeSource->Node(), fTimeSource->RealTime());
+	*/
 }
 
 /*************************************************************
@@ -575,6 +593,10 @@ BMediaNode::HandleMessage(int32 message,
 			status_t rv;
 			printf("NODE_SET_TIMESOURCE, node %ld, timesource %ld\n", fNodeID, command->timesource_id);
 			roster = BMediaRoster::Roster();
+			
+			// Time sources are not reference counted. But since
+			// BMediaRoster::GetNodeFor() will create a clone of
+			// any node, we will need to release it!
 			rv = roster->GetNodeFor(command->timesource_id, &clone);
 			if (rv != B_OK) {
 				FATAL("NODE_SET_TIMESOURCE: Error, GetNodeFor failed\n");
@@ -583,19 +605,31 @@ BMediaNode::HandleMessage(int32 message,
 			newsource = roster->MakeTimeSourceFor(clone);
 			if (newsource == 0) {
 				FATAL("NODE_SET_TIMESOURCE: Error, MakeTimeSourceFor failed\n");
-				roster->ReleaseNode(clone);
+				roster->ReleaseNode(clone); // release cloned node
 				return B_OK;
 			}
-			roster->ReleaseNode(clone);
+			roster->ReleaseNode(clone); // release cloned node
 			if (fTimeSource) {
+				// as this node already had a timesource, we need
+				// we need to remove this node from time source control
 				fTimeSource->RemoveMe(this);
+				// Then released the time source
 				fTimeSource->Release();
+				// now the new one is assigned
 				fTimeSource = newsource;
+				// we add this node to the time source controll
+				fTimeSource->AddMe(this);
+				// and call the SetTimeSource hook function to notify
+				// any derived class
 				SetTimeSource(fTimeSource);
 			} else {
+				// the new time source is assigned to this node
 				fTimeSource = newsource;
+				// we add this node to the time source controll
+				fTimeSource->AddMe(this);
 			}
-			fTimeSource->AddMe(this);
+			printf("#### BMediaNode::HandleMessage NODE_SET_TIMESOURCE: node %ld has been assigned time source %ld\n", ID(), fTimeSource->ID());
+			
 			//roster->StartTimeSource(fTimeSource->Node(), fTimeSource->RealTime());
 			return B_OK;
 		}
@@ -839,7 +873,7 @@ BMediaNode::_InitObject(const char *name, media_node_id id, uint64 kinds)
 	fTimeSourceThis = 0;
 
 	// create control port
-	fControlPort = create_port(64,fName);
+	fControlPort = create_port(64, fName);
 }
 
 

src/kits/media/MediaRoster.cpp

@@ -298,7 +298,22 @@ status_t
 BMediaRoster::GetTimeSource(media_node * out_node)
 {
 	CALLED();
-	return MediaRosterEx(this)->GetNode(TIME_SOURCE, out_node);
+	status_t rv;
+
+	// XXX need to do this in a nicer way.
+
+	rv = MediaRosterEx(this)->GetNode(TIME_SOURCE, out_node); 
+	if (rv != B_OK)
+		return rv;
+		
+	// We don't do reference counting for timesources, that's why we
+	// release the node immediately.
+	ReleaseNode(*out_node);
+
+	// we need to remember to not use this node with server side reference counting
+	out_node->kind |= NODE_KIND_NO_REFCOUNTING;
+	
+	return B_OK;
 }
 
 
@@ -404,7 +419,22 @@ status_t
 BMediaRoster::GetSystemTimeSource(media_node * clone)
 {
 	CALLED();
-	return MediaRosterEx(this)->GetNode(SYSTEM_TIME_SOURCE, clone);
+	status_t rv;
+
+	// XXX need to do this in a nicer way.
+
+	rv = MediaRosterEx(this)->GetNode(SYSTEM_TIME_SOURCE, clone); 
+	if (rv != B_OK)
+		return rv;
+		
+	// We don't do reference counting for timesources, that's why we
+	// release the node immediately.
+	ReleaseNode(*clone);
+
+	// we need to remember to not use this node with server side reference counting
+	clone->kind |= NODE_KIND_NO_REFCOUNTING;
+	
+	return B_OK;
 }
 
 
@@ -414,6 +444,11 @@ BMediaRoster::ReleaseNode(const media_node & node)
 	CALLED();
 	if (IS_INVALID_NODE(node))
 		return B_MEDIA_BAD_NODE;
+		
+	if (node.kind & NODE_KIND_NO_REFCOUNTING) {
+		printf("BMediaRoster::ReleaseNode, trying to release reference counting disabled timesource, node %ld, port %ld, team %ld\n", node.node, node.port, team);
+		return B_OK;
+	}
 
 	server_release_node_request request;
 	server_release_node_reply reply;
@@ -447,7 +482,7 @@ BMediaRoster::MakeTimeSourceFor(const media_node & for_node)
 		media_node clone;
 		GetSystemTimeSource(&clone);
 		source = _TimeSourceObjectManager->GetTimeSource(clone);
-		ReleaseNode(clone);
+//		ReleaseNode(clone);
 	} else {
 		source = _TimeSourceObjectManager->GetTimeSource(for_node);
 	}
@@ -1404,6 +1439,11 @@ BMediaRoster::UnregisterNode(BMediaNode * node)
 	CALLED();
 	if (node == NULL)
 		return B_BAD_VALUE;
+
+	if (node->fKinds & NODE_KIND_NO_REFCOUNTING) {
+		printf("BMediaRoster::UnregisterNode, trying to unregister reference counting disabled timesource, node %ld, port %ld, team %ld\n", node->ID(), node->ControlPort(), team);
+		return B_OK;
+	}
 	
 	if (node->fRefCount != 0) {
 		FATAL("BMediaRoster::UnregisterNode: Warning node id %ld, name '%s' has local reference count of %ld\n", node->ID(), node->Name(), node->fRefCount);
@@ -1485,12 +1525,17 @@ BMediaRoster::SetTimeSourceFor(media_node_id node,
 	media_node clone;
 	status_t rv, result;
 	
+	// we need to get a clone of the node to have a port id
 	rv = GetNodeFor(node, &clone);
 	if (rv != B_OK) {
 		FATAL("BMediaRoster::SetTimeSourceFor, GetNodeFor failed, node id %ld\n", node);
 		return B_ERROR;
 	}
+
+	printf("#### BMediaRoster::SetTimeSourceFor: node %ld will be assigned time source %ld\n", node, time_source);
 	
+	// we just send the request to set time_source-id as timesource to the node,
+	// the NODE_SET_TIMESOURCE handler code will do the real assignment
 	result = B_OK;
 	node_set_timesource_command cmd;
 	cmd.timesource_id = time_source;
@@ -1500,6 +1545,7 @@ BMediaRoster::SetTimeSourceFor(media_node_id node,
 		result = B_ERROR;
 	}
 
+	// we release the colone
 	rv = ReleaseNode(clone);
 	if (rv != B_OK) {
 		FATAL("BMediaRoster::SetTimeSourceFor, ReleaseNode failed, node id %ld\n", node);
@@ -1732,7 +1778,7 @@ BMediaRoster::InstantiateDormantNode(const dormant_node_info & in_info,
 		FATAL("BMediaRoster::InstantiateDormantNode Error: requested B_FLAVOR_IS_GLOBAL, but dormant node has B_FLAVOR_IS_LOCAL\n");
 		return B_BAD_VALUE;
 	}
-#if 0
+//#if 0
 	// If either the node, or the caller requested to make the instance global
 	// we will do it by forwarding this request into the media_addon_server, which
 	// in turn will call BMediaRosterEx::InstantiateDormantNode to create the node
@@ -1758,8 +1804,8 @@ BMediaRoster::InstantiateDormantNode(const dormant_node_info & in_info,
 	
 		return MediaRosterEx(this)->InstantiateDormantNode(in_info.addon, in_info.flavor_id, out_node);
 	}
-#endif
+//#endif
-	return MediaRosterEx(this)->InstantiateDormantNode(in_info.addon, in_info.flavor_id, out_node);
+//	return MediaRosterEx(this)->InstantiateDormantNode(in_info.addon, in_info.flavor_id, out_node);
 }
 
 									 

src/kits/media/TimeSourceObject.cpp

@@ -10,6 +10,7 @@
 #include <OS.h>
 #include <stdio.h>
 #include <MediaRoster.h>
+#include "MediaMisc.h"
 #include "TimeSourceObject.h"
 #include "TimeSourceObjectManager.h"
 
@@ -32,7 +33,9 @@ TimeSourceObject::TimeSourceObject(const media_node &node)
 	}
 
 	AddNodeKind(NODE_KIND_SHADOW_TIMESOURCE);
-	fControlPort = -1234;
+	AddNodeKind(NODE_KIND_NO_REFCOUNTING);
+	fControlPort = SHADOW_TIMESOURCE_CONTROL_PORT; // XXX if we don't do this, we get a infinite loop somewhere. This needs to be debugged
+	
 //	printf("TimeSourceObject::TimeSourceObject leave, node id %ld\n", fNodeID);
 }
 
@@ -71,6 +74,7 @@ SystemTimeSourceObject::SystemTimeSourceObject(const media_node &node)
 //	printf("SystemTimeSourceObject::SystemTimeSourceObject enter, id = %ld\n", id);
 	fIsRealtime = true;
 	AddNodeKind(NODE_KIND_SYSTEM_TIMESOURCE);
+	AddNodeKind(NODE_KIND_NO_REFCOUNTING);
 //	printf("SystemTimeSourceObject::SystemTimeSourceObject leave, node id %ld\n", ID());
 }
 

src/kits/media/TimeSourceObjectManager.cpp

@@ -2,6 +2,9 @@
  * Copyright (c) 2002 Marcus Overhagen. All Rights Reserved.
  * This file may be used under the terms of the MIT License.
  *
+ * This works like a cache for time source objects, to make sure
+ * each team only has one object representation for each time source.
+ *
  ***********************************************************************/
 
 #include <OS.h>
@@ -47,6 +50,10 @@ TimeSourceObjectManager::~TimeSourceObjectManager()
 	delete fMap;
 }
 
+/* BMediaRoster::MakeTimeSourceFor does use this function to request
+ * a time source object. If it is already in memory, it will be
+ * Acquired(), if not, a new TimeSourceObject will be created.
+ */
 BTimeSource *
 TimeSourceObjectManager::GetTimeSource(const media_node &node)
 {
@@ -74,7 +81,8 @@ TimeSourceObjectManager::GetTimeSource(const media_node &node)
 	BTimeSource **pts;
 	if (fMap->Get(node.node, &pts))
 		return dynamic_cast<BTimeSource *>((*pts)->Acquire());
-		
+
+/*		
 	media_node clone;
 	status_t rv;
 	
@@ -88,8 +96,18 @@ TimeSourceObjectManager::GetTimeSource(const media_node &node)
 	ts = new TimeSourceObject(clone);
 	fMap->Insert(clone.node, ts);
 	return ts;
+*/
+	// time sources are not accounted in node reference counting
+	BTimeSource *ts;
+	ts = new TimeSourceObject(node);
+	fMap->Insert(node.node, ts);
+	return ts;
 }
 
+/* This function is called during deletion of the time source object.
+ *
+ * I'm not sure if there is a race condition with the function above.
+ */
 void
 TimeSourceObjectManager::ObjectDeleted(BTimeSource *timesource)
 {