From 4aa9ff58c6392bee9f9dd7b4ef371d1bcd733cb1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= <superstippi@gmx.de>
Date: Sat, 31 May 2008 10:39:45 +0000
Subject: [PATCH] Fix a locking problem with inner locks to protect the race
 condition when the producer media nodes would access the suppliers in their
 own thread without having any locks held, while the window could replace the
 suppliers. I think since I delayed the deletion of the suppliers in the
 controllers, this problem was only theoretical... but this is just more
 clean.

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25734 a95241bf-73f2-0310-859d-f6bbb57e9c96
---
 .../media_node_framework/video/VideoProducer.cpp    | 13 ++++---------
 .../mediaplayer/supplier/ProxyAudioSupplier.cpp     | 10 +++++++++-
 src/apps/mediaplayer/supplier/ProxyAudioSupplier.h  |  3 +++
 .../mediaplayer/supplier/ProxyVideoSupplier.cpp     |  6 +++++-
 src/apps/mediaplayer/supplier/ProxyVideoSupplier.h  |  3 +++
 5 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp b/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp
index e39479f208..96783ab517 100644
--- a/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp
+++ b/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp
@@ -791,17 +791,12 @@ h->start_time = 0;
 							"playlistFrame: %Ld\n", fFrame, playlistFrame);
 						bool forceOrWasCached = forceSendingBuffer;
 	
-						if (fManager->LockWithTimeout(5000) == B_OK) {
-							// we need to lock the manager, or our
-							// fSupplier might work on bad data
-							err = fSupplier->FillBuffer(playlistFrame,
-								buffer->Data(), &mf, forceOrWasCached);
-							fManager->Unlock();
-						} else {
-							err = B_ERROR;
-						}
+						err = fSupplier->FillBuffer(playlistFrame,
+							buffer->Data(), &mf, forceOrWasCached);
 						// clean the buffer if something went wrong
 						if (err != B_OK) {
+							// TODO: should use "back value" according
+							// to color space!
 							memset(buffer->Data(), 0, h->size_used);
 							err = B_OK;
 						}
diff --git a/src/apps/mediaplayer/supplier/ProxyAudioSupplier.cpp b/src/apps/mediaplayer/supplier/ProxyAudioSupplier.cpp
index 761551a053..5f2c5a9d53 100644
--- a/src/apps/mediaplayer/supplier/ProxyAudioSupplier.cpp
+++ b/src/apps/mediaplayer/supplier/ProxyAudioSupplier.cpp
@@ -43,7 +43,9 @@ struct PlayingInterval {
 
 
 ProxyAudioSupplier::ProxyAudioSupplier(PlaybackManager* playbackManager)
-	: fPlaybackManager(playbackManager)
+	: fSupplierLock("audio supplier lock")
+
+	, fPlaybackManager(playbackManager)
 	, fVideoFrameRate(25.0)
 
 	, fSupplier(NULL)
@@ -98,6 +100,8 @@ ProxyAudioSupplier::GetFrames(void* buffer, int64 frameCount,
 		TRACE("GetFrames() - LOCKING THE PLAYBACK MANAGER TIMED OUT!!!\n");
 	}
 
+	BAutolock _(fSupplierLock);
+
 	// retrieve the audio data for each interval.
 	int64 framesRead = 0;
 	while (!playingIntervals.IsEmpty()) {
@@ -165,6 +169,8 @@ ProxyAudioSupplier::SetFormat(const media_format& format)
 		TRACE("SetFormat(%s)\n", string);
 	#endif
 
+	BAutolock _(fSupplierLock);
+
 	fAudioResampler.SetFormat(format);
 
 	// In case SetSupplier was called before, we need
@@ -199,6 +205,8 @@ ProxyAudioSupplier::SetSupplier(AudioTrackSupplier* supplier,
 //videoFrameRate);
 	TRACE("SetSupplier(%p, %.1f)\n", supplier, videoFrameRate);
 
+	BAutolock _(fSupplierLock);
+
 	fSupplier = supplier;
 	fVideoFrameRate = videoFrameRate;
 
diff --git a/src/apps/mediaplayer/supplier/ProxyAudioSupplier.h b/src/apps/mediaplayer/supplier/ProxyAudioSupplier.h
index e8194cb8d9..b265ff9d35 100644
--- a/src/apps/mediaplayer/supplier/ProxyAudioSupplier.h
+++ b/src/apps/mediaplayer/supplier/ProxyAudioSupplier.h
@@ -5,6 +5,7 @@
 #ifndef PROXY_AUDIO_SUPPLIER_H
 #define PROXY_AUDIO_SUPPLIER_H
 
+#include <Locker.h>
 
 #include "AudioResampler.h"
 #include "AudioSupplier.h"
@@ -45,6 +46,8 @@ private:
 			void*				_SkipFrames(void* buffer, int64 frames) const;
 
 private:
+	BLocker						fSupplierLock;
+
 	PlaybackManager*			fPlaybackManager;
 	float						fVideoFrameRate;
 
diff --git a/src/apps/mediaplayer/supplier/ProxyVideoSupplier.cpp b/src/apps/mediaplayer/supplier/ProxyVideoSupplier.cpp
index 2fd0ce0e9d..a03f9f8c61 100644
--- a/src/apps/mediaplayer/supplier/ProxyVideoSupplier.cpp
+++ b/src/apps/mediaplayer/supplier/ProxyVideoSupplier.cpp
@@ -12,7 +12,8 @@
 
 
 ProxyVideoSupplier::ProxyVideoSupplier()
-	: fSupplier(NULL)
+	: fSupplierLock("video supplier lock")
+	, fSupplier(NULL)
 {
 }
 
@@ -26,6 +27,7 @@ status_t
 ProxyVideoSupplier::FillBuffer(int64 startFrame, void* buffer,
 	const media_format* format, bool& wasCached)
 {
+	BAutolock _(fSupplierLock);
 //printf("ProxyVideoSupplier::FillBuffer(%lld)\n", startFrame);
 	if (fSupplier == NULL)
 		return B_NO_INIT;
@@ -60,6 +62,8 @@ ProxyVideoSupplier::DeleteCaches()
 void
 ProxyVideoSupplier::SetSupplier(VideoTrackSupplier* supplier)
 {
+	BAutolock _(fSupplierLock);
+
 	fSupplier = supplier;
 }
 
diff --git a/src/apps/mediaplayer/supplier/ProxyVideoSupplier.h b/src/apps/mediaplayer/supplier/ProxyVideoSupplier.h
index 505989c387..2bf41f1352 100644
--- a/src/apps/mediaplayer/supplier/ProxyVideoSupplier.h
+++ b/src/apps/mediaplayer/supplier/ProxyVideoSupplier.h
@@ -5,6 +5,7 @@
 #ifndef PROXY_VIDEO_SUPPLIER_H
 #define PROXY_VIDEO_SUPPLIER_H
 
+#include <Locker.h>
 
 #include "VideoSupplier.h"
 
@@ -26,6 +27,8 @@ public:
 			void				SetSupplier(VideoTrackSupplier* supplier);
 
 private:
+			BLocker				fSupplierLock;
+
 			VideoTrackSupplier*	fSupplier;
 };