MediaConnection: Review format negotiation mechanism

headers/private/media/experimental/MediaClientDefs.h

@@ -67,8 +67,12 @@ typedef struct media_connection {
 	BMessage*					ToMessage() const;
 
 private:
-	media_input					_MediaInput() const;
+	// NOTE: We are doing this on purpose to avoid redundancy we
-	media_output				_MediaOutput() const;
+	// want to build the input/output on the fly. In pratice, the
+	// only thing that can change is the format which is kept updated
+	// to reflect the current status of this connection.
+	media_input					_BuildMediaInput() const;
+	media_output				_BuildMediaOutput() const;
 
 	const media_source&			_Source() const;
 	const media_destination&	_Destination() const;
@@ -84,6 +88,9 @@ private:
 
     media_source				source;
     media_destination			destination;
+
+    // This format always reflect the most updated format depending
+    // on the connection phase.
     media_format				format;
     char						name[B_MEDIA_NAME_LENGTH];
 

headers/private/media/experimental/MediaConnection.h

@@ -43,12 +43,6 @@ public:
 	// return media_connection::null otherwise.
 	media_connection				Endpoint();
 
-	// This allow to specify a format that will be used while
-	// connecting to another node.
-	void							SetAcceptedFormat(
-										const media_format& format);
-	const media_format&				AcceptedFormat() const;
-
 	// Represents the buffer size, implement it to return the buffer size
 	// you decided for this connection.
 	// TODO: Do we want this (and ChainSize) moved on the output side?
@@ -125,13 +119,14 @@ protected:
 	virtual							~BMediaInput();
 
 	// Callbacks
+	virtual status_t				AcceptFormat(media_format* format) = 0;
+
 	virtual void					HandleBuffer(BBuffer* buffer);
 
 	virtual void					Connected(const media_format& format);
 	virtual void					Disconnected();
 
 private:
-	media_input						_MediaInput() const;
 
 	virtual	void					_ReservedMediaInput0();
 	virtual	void					_ReservedMediaInput1();
@@ -156,8 +151,8 @@ protected:
 	virtual							~BMediaOutput();
 
 	// Callbacks
-	virtual status_t				PrepareToConnect(media_format* format);
+	virtual status_t				PrepareToConnect(media_format* format) = 0;
-	virtual status_t				FormatProposal(media_format* format);
+	virtual status_t				FormatProposal(media_format* format) = 0;
 
 	// When a connection is not binded with another, and you really don't want
 	// to use BMediaGraph it's your job to send the buffer to the connection
@@ -170,7 +165,6 @@ protected:
 	virtual void					Disconnected();
 
 private:
-	media_output					_MediaOutput() const;
 
 	// TODO: possibly unneeded.
 	void							_SetEnabled(bool enabled);

headers/private/media/experimental/SimpleMediaClient.h

@@ -117,6 +117,12 @@ public:
 	virtual size_t					BufferSize() const;
 	virtual void					SetBufferSize(size_t bufferSize);
 
+			// This allow to specify a format that will be used while
+			// connecting to another node.
+			void					SetAcceptedFormat(
+										const media_format& format);
+			const media_format&		AcceptedFormat() const;
+
 protected:
 									BSimpleMediaConnection(
 										media_connection_kinds kinds);
@@ -128,6 +134,8 @@ protected:
 			void*					fBufferCookie;
 
 			size_t					fBufferSize;
+
+			media_format			fAcceptedFormat;
 };
 
 
@@ -138,10 +146,12 @@ public:
 protected:
 	virtual							~BSimpleMediaInput();
 
-	virtual void					Connected(const media_format& format);
+	virtual status_t				AcceptFormat(media_format* format);
-	virtual void					Disconnected();
 
 	virtual void					HandleBuffer(BBuffer* buffer);
+
+	virtual void					Connected(const media_format& format);
+	virtual void					Disconnected();
 };
 
 
@@ -152,10 +162,11 @@ public:
 protected:
 	virtual							~BSimpleMediaOutput();
 
+	virtual status_t				PrepareToConnect(media_format* format);
+	virtual status_t				FormatProposal(media_format* format);
+
 	virtual void					Connected(const media_format& format);
 	virtual void					Disconnected();
-
-	virtual status_t				FormatProposal(media_format* format);
 };
 
 

src/kits/media/experimental/MediaClient.cpp

@@ -513,13 +513,24 @@ BMediaClient::_ConnectInput(BMediaOutput* output,
 	if (input._Destination() == media_destination::null)
 		return B_MEDIA_BAD_DESTINATION;
 
-	media_output ourOutput = output->Connection()._MediaOutput();
+	media_output ourOutput = output->Connection()._BuildMediaOutput();
-	media_input theirInput = input._MediaInput();
+	media_input theirInput = input._BuildMediaInput();
-	media_format format = output->AcceptedFormat();
+	media_format format;
 
-	return BMediaRoster::CurrentRoster()->Connect(ourOutput.source,
+	// NOTE: We want to set this data in the callbacks if possible.
+	// The correct format should have been set in BMediaConnection::Connected.
+	// TODO: Perhaps add some check assert?
+
+	status_t ret = BMediaRoster::CurrentRoster()->Connect(ourOutput.source,
 		theirInput.destination, &format, &ourOutput, &theirInput,
 		BMediaRoster::B_CONNECT_MUTED);
+
+#if 0
+	if (ret == B_OK)
+		output->fConnection.format = format;
+#endif
+
+	return ret;
 }
 
 
@@ -532,13 +543,24 @@ BMediaClient::_ConnectOutput(BMediaInput* input,
 	if (output._Source() == media_source::null)
 		return B_MEDIA_BAD_SOURCE;
 
-	media_input ourInput = input->Connection()._MediaInput();
+	media_input ourInput = input->Connection()._BuildMediaInput();
-	media_output theirOutput = output._MediaOutput();
+	media_output theirOutput = output._BuildMediaOutput();
-	media_format format = input->AcceptedFormat();
+	media_format format;
 
-	return BMediaRoster::CurrentRoster()->Connect(theirOutput.source,
+	// NOTE: We want to set this data in the callbacks if possible.
+	// The correct format should have been set in BMediaConnection::Connected.
+	// TODO: Perhaps add some check assert?
+
+	status_t ret = BMediaRoster::CurrentRoster()->Connect(theirOutput.source,
 		ourInput.destination, &format, &theirOutput, &ourInput,
 		BMediaRoster::B_CONNECT_MUTED);
+
+#if 0
+	if (ret == B_OK)
+		input->fConnection.format = format;
+#endif
+
+	return ret;
 }
 
 

src/kits/media/experimental/MediaClientDefs.cpp

@@ -59,7 +59,7 @@ media_connection::IsOutput() const
 
 
 media_input
-media_connection::_MediaInput() const
+media_connection::_BuildMediaInput() const
 {
 	media_input input;
 	input.node = client.node;
@@ -71,7 +71,7 @@ media_connection::_MediaInput() const
 
 
 media_output
-media_connection::_MediaOutput() const
+media_connection::_BuildMediaOutput() const
 {
 	media_output output;
 	output.node = client.node;

src/kits/media/experimental/MediaClientNode.cpp

@@ -149,12 +149,7 @@ BMediaClientNode::AcceptFormat(const media_destination& dest,
 	if (conn == NULL)
 		return B_MEDIA_BAD_DESTINATION;
 
-	if (format_is_compatible(*format, conn->AcceptedFormat()))
+	return conn->AcceptFormat(format);
-		return B_OK;
-
-	*format = conn->AcceptedFormat();
-
-	return B_MEDIA_BAD_FORMAT;
 }
 
 
@@ -173,7 +168,7 @@ BMediaClientNode::GetNextInput(int32* cookie,
 	} else {
 		BMediaInput* conn = fOwner->InputAt(*cookie);
 		if (conn != NULL) {
-			*input = conn->_MediaInput();
+			*input = conn->fConnection._BuildMediaInput();
 			*cookie += 1;
 			return B_OK;
 		}
@@ -228,11 +223,11 @@ BMediaClientNode::Connected(const media_source& source,
 		return B_MEDIA_BAD_DESTINATION;
 
 	conn->fConnection.source = source;
-	conn->SetAcceptedFormat(format);
+	conn->fConnection.format = format;
 
 	conn->Connected(format);
 
-	*outInput = conn->_MediaInput();
+	*outInput = conn->fConnection._BuildMediaInput();
 	return B_OK;
 }
 
@@ -336,7 +331,7 @@ BMediaClientNode::GetNextOutput(int32* cookie, media_output* output)
 	} else {
 		BMediaOutput* conn = fOwner->OutputAt(*cookie);
 		if (conn != NULL) {
-			*output = conn->_MediaOutput();
+			*output = conn->fConnection._BuildMediaOutput();
 			*cookie += 1;
 			return B_OK;
 		}
@@ -431,7 +426,8 @@ BMediaClientNode::Connect(status_t status, const media_source& source,
 	}
 
 	conn->fConnection.destination = dest;
-	conn->SetAcceptedFormat(format);
+	conn->fConnection.format = format;
+
 	strcpy(name, Name());
 
 	// TODO: add correct latency estimate
@@ -627,7 +623,7 @@ BMediaClientNode::_ProduceNewBuffer(const media_timed_event* event,
 	}
 
 	bigtime_t time = 0;
-	media_format format = output->AcceptedFormat();
+	media_format format = output->fConnection.format;
 	if (format.IsAudio()) {
 		size_t nFrames = format.u.raw_audio.buffer_size
 			/ ((format.u.raw_audio.format
@@ -656,7 +652,7 @@ BMediaClientNode::_GetNextBuffer(BMediaOutput* output, bigtime_t eventTime)
 	}
 
 	media_header* header = buffer->Header();
-	header->type = output->AcceptedFormat().type;
+	header->type = output->fConnection.format.type;
 	header->size_used = output->BufferSize();
 	header->time_source = TimeSource()->ID();
 	header->start_time = eventTime;

src/kits/media/experimental/MediaConnection.cpp

@@ -62,24 +62,6 @@ BMediaConnection::Binding() const
 }
 
 
-void
-BMediaConnection::SetAcceptedFormat(const media_format& format)
-{
-	CALLED();
-
-	fConnection.format = format;
-}
-
-
-const media_format&
-BMediaConnection::AcceptedFormat() const
-{
-	CALLED();
-
-	return fConnection.format;
-}
-
-
 bool
 BMediaConnection::IsConnected() const
 {
@@ -115,6 +97,9 @@ BMediaConnection::Release()
 void
 BMediaConnection::Connected(const media_format& format)
 {
+	// Update the status of our connection format.
+	fConnection.format = format;
+
 	fConnected = true;
 }
 
@@ -209,13 +194,6 @@ BMediaInput::Disconnected()
 }
 
 
-media_input
-BMediaInput::_MediaInput() const
-{
-	return Connection()._MediaInput();
-}
-
-
 void BMediaInput::_ReservedMediaInput0() {}
 void BMediaInput::_ReservedMediaInput1() {}
 void BMediaInput::_ReservedMediaInput2() {}
@@ -243,42 +221,6 @@ BMediaOutput::~BMediaOutput()
 }
 
 
-bool
-BMediaOutput::_IsEnabled() const
-{
-	CALLED();
-
-	return fEnabled;
-}
-
-
-void
-BMediaOutput::_SetEnabled(bool enabled)
-{
-	fEnabled = enabled;
-}
-
-
-status_t
-BMediaOutput::PrepareToConnect(media_format* format)
-{
-	if (!format_is_compatible(AcceptedFormat(), *format))
-		return B_ERROR;
-
-	SetAcceptedFormat(*format);
-
-	return B_OK;
-}
-
-
-status_t
-BMediaOutput::FormatProposal(media_format* format)
-{
-	*format = AcceptedFormat();
-	return B_OK;
-}
-
-
 status_t
 BMediaOutput::SendBuffer(BBuffer* buffer)
 {
@@ -308,10 +250,19 @@ BMediaOutput::Disconnected()
 }
 
 
-media_output
+bool
-BMediaOutput::_MediaOutput() const
+BMediaOutput::_IsEnabled() const
 {
-	return Connection()._MediaOutput();
+	CALLED();
+
+	return fEnabled;
+}
+
+
+void
+BMediaOutput::_SetEnabled(bool enabled)
+{
+	fEnabled = enabled;
 }
 
 

src/kits/media/experimental/SimpleMediaClient.cpp

@@ -164,6 +164,24 @@ BSimpleMediaConnection::SetBufferSize(size_t bufferSize)
 }
 
 
+void
+BSimpleMediaConnection::SetAcceptedFormat(const media_format& format)
+{
+	CALLED();
+
+	fAcceptedFormat = format;
+}
+
+
+const media_format&
+BSimpleMediaConnection::AcceptedFormat() const
+{
+	CALLED();
+
+	return fAcceptedFormat;
+}
+
+
 BSimpleMediaInput::BSimpleMediaInput()
 	:
 	BMediaConnection(B_MEDIA_INPUT),
@@ -179,12 +197,30 @@ BSimpleMediaInput::~BSimpleMediaInput()
 }
 
 
+status_t
+BSimpleMediaInput::AcceptFormat(media_format* format)
+{
+	CALLED();
+
+	// TODO: Add hooks
+
+	if (format_is_compatible(*format, AcceptedFormat()))
+		return B_OK;
+
+	*format = AcceptedFormat();
+
+	return B_MEDIA_BAD_FORMAT;
+}
+
+
 void
 BSimpleMediaInput::Connected(const media_format& format)
 {
 	if (fNotifyHook != NULL)
 		(*fNotifyHook)(this, BSimpleMediaConnection::B_INPUT_CONNECTED);
 
+	SetAcceptedFormat(format);
+
 	BMediaInput::Connected(format);
 }
 
@@ -224,15 +260,28 @@ BSimpleMediaOutput::~BSimpleMediaOutput()
 }
 
 
+status_t
+BSimpleMediaOutput::PrepareToConnect(media_format* format)
+{
+	// TODO: Add hooks
+
+	if (!format_is_compatible(AcceptedFormat(), *format))
+		return B_ERROR;
+
+	return B_OK;
+}
+
+
 status_t
 BSimpleMediaOutput::FormatProposal(media_format* format)
 {
 	if (fNotifyHook != NULL) {
 		return (*fNotifyHook)(this,
 			BSimpleMediaConnection::B_FORMAT_PROPOSAL, format);
-	}
+	} else
+		*format = AcceptedFormat();
 
-	return BMediaOutput::FormatProposal(format);
+	return B_OK;
 }
 
 
@@ -242,6 +291,8 @@ BSimpleMediaOutput::Connected(const media_format& format)
 	if (fNotifyHook != NULL)
 		(*fNotifyHook)(this, BSimpleMediaConnection::B_OUTPUT_CONNECTED);
 
+	SetAcceptedFormat(format);
+
 	BMediaOutput::Connected(format);
 }