changeset 24:7d28bed0864e

* Rearrange Python plugin construction.

  Formerly, the PyPluginAdapter has retained a single plugin instance pointer
  for each plugin found, and its createPlugin method has simply returned a new
  PyPlugin object wrapping the same instance pointer.  This has a couple of
  negative consequences:
   - Because construction of the actual Python instance occurred before the
     wrapper was constructed, it was not possible to pass arguments (i.e.
     the sample rate) from the wrapper constructor to the Python plugin
     instance constructor -- they had to be passed later, to initialise,
     disadvantaging those plugins that would like to use the sample rate
     for parameter & step/block size calculations etc
   - Because there was only a single Python plugin instance, it was not
     possible to run more than one instance at once with any isolation

  This rework instead stores the Python class pointer (rather than instance
  pointer) in the PyPluginAdapter, and each PyPlugin wrapper instance creates
  its own Python plugin instance.  What could possibly go wrong?

--- a/Example VamPy plugins/PySpectralCentroid.py	Thu Jul 16 13:19:20 2009 +0000
+++ b/Example VamPy plugins/PySpectralCentroid.py	Mon Aug 17 15:22:06 2009 +0000
@@ -5,19 +5,18 @@
 
 class PySpectralCentroid: 
 	
-	def __init__(self): 
-		self.m_imputSampleRate = 0.0 
+	def __init__(self,inputSampleRate): 
+		self.m_inputSampleRate = inputSampleRate
 		self.m_stepSize = 0
 		self.m_blockSize = 0
 		self.m_channels = 0
 		self.previousSample = 0.0
 		self.threshold = 0.05
 		
-	def initialise(self,channels,stepSize,blockSize,inputSampleRate):
+	def initialise(self,channels,stepSize,blockSize):
 		self.m_channels = channels
 		self.m_stepSize = stepSize		
 		self.m_blockSize = blockSize
-		self.m_inputSampleRate = inputSampleRate
 		return True
 	
 	def getMaker(self):

--- a/Example VamPy plugins/PySpectralFeatures.py	Thu Jul 16 13:19:20 2009 +0000
+++ b/Example VamPy plugins/PySpectralFeatures.py	Mon Aug 17 15:22:06 2009 +0000
@@ -5,19 +5,18 @@
 
 class PySpectralFeatures: 
 	
-	def __init__(self): 
-		self.m_imputSampleRate = 0.0 
+	def __init__(self,inputSampleRate): 
+		self.m_inputSampleRate = inputSampleRate
 		self.m_stepSize = 0
 		self.m_blockSize = 0
 		self.m_channels = 0
 		self.threshold = 0.00
 		self.r = 2.0
 		
-	def initialise(self,channels,stepSize,blockSize,inputSampleRate):
+	def initialise(self,channels,stepSize,blockSize):
 		self.m_channels = channels
 		self.m_stepSize = stepSize		
 		self.m_blockSize = blockSize
-		self.m_inputSampleRate = inputSampleRate
 		#self.prevMag = ones((blockSize/2)-1) / ((blockSize/2)-1)
 		self.prevMag = zeros((blockSize/2)-1)
 		self.prevMag[0] = 1

--- a/Example VamPy plugins/PyZeroCrossing.py	Thu Jul 16 13:19:20 2009 +0000
+++ b/Example VamPy plugins/PyZeroCrossing.py	Mon Aug 17 15:22:06 2009 +0000
@@ -1,21 +1,24 @@
 '''PyZeroCrossing.py - Example plugin demonstrates''' 
 '''how to call a python class using the VamPy Vamp plugin'''
 
+from random import *
+
 class PyZeroCrossing: 
 	
-	def __init__(self): 
-		self.m_imputSampleRate = 0.0 
+	def __init__(self,inputSampleRate): 
+		self.m_inputSampleRate = inputSampleRate 
 		self.m_stepSize = 0
 		self.m_blockSize = 0
 		self.m_channels = 0
 		self.previousSample = 0.0
 		self.threshold = 0.005
+		self.identity = random()
+		self.counter = 0
 		
-	def initialise(self,channels,stepSize,blockSize,inputSampleRate):
+	def initialise(self,channels,stepSize,blockSize):
 		self.m_channels = channels
 		self.m_stepSize = stepSize		
 		self.m_blockSize = blockSize
-		self.m_inputSampleRate = inputSampleRate
 		return True
 	
 	def getMaker(self):
@@ -70,8 +73,8 @@
 	def getParameterDescriptors(self):
 		paramlist1={
 		'identifier':'threshold',
-		'name':'Noise threshold: ',
-		'description':'Return null or delete this function if not needed.',
+		'name':'Noise threshold',
+		'description':'',
 		'unit':'v',
 		'minValue':0.0,
 		'maxValue':0.5,
@@ -97,6 +100,9 @@
 		count = 0.0;
 		channel = inbuf[0]
 
+		print "Identity ", self.identity, ", counter ", self.counter
+		self.counter = self.counter + 1
+
 		#we have two outputs defined thus we have to declare
 		#them as empty dictionaries in our output list
 		#in order to be able to return variable rate outputs

--- a/Makefile.cc-linux	Thu Jul 16 13:19:20 2009 +0000
+++ b/Makefile.cc-linux	Mon Aug 17 15:22:06 2009 +0000
@@ -1,8 +1,8 @@
 
-CXXFLAGS	:= -I../vamp-plugin-sdk -O2 -Wall -I/usr/include/python2.6 -fPIC
+CXXFLAGS	:= -I../vamp-plugin-sdk -O0 -g -Wall -I/usr/include/python2.6 -fPIC
 
 vampy.so:	PyPlugin.o PyPlugScanner.o pyvamp-main.o Mutex.o
-	g++ -shared $^ -o $@ -L../vamp-plugin-sdk/vamp-sdk -Wl,-Bstatic -lvamp-sdk -Wl,-Bdynamic -lpython2.6 -lpthread -Wl,--version-script=vamp-plugin.map
+	g++ -shared $^ -o $@ -L../vamp-plugin-sdk/src -Wl,-Bstatic -lvamp-sdk -Wl,-Bdynamic -lpython2.6 -lpthread -Wl,--version-script=vamp-plugin.map
 
 clean:	
 	rm *.o

--- a/PyPlugScanner.cpp	Thu Jul 16 13:19:20 2009 +0000
+++ b/PyPlugScanner.cpp	Mon Aug 17 15:22:06 2009 +0000
@@ -62,7 +62,7 @@
 	
 	vector<string> pyPlugs;
 	string pluginKey;
-	PyObject *pyClassInstance;
+	PyObject *pyClass;
 	
     for (size_t i = 0; i < m_path.size(); ++i) {
         
@@ -74,13 +74,13 @@
 				if (!script.empty()) {					
 					string classname=script.substr(0,script.rfind('.'));
 					pluginKey=joinPath(m_path[i],script)+":"+classname;
-					pyClassInstance = getScriptInstance(m_path[i],classname);
-					if (pyClassInstance == NULL) 
+					pyClass = getScriptClass(m_path[i],classname);
+					if (pyClass == NULL) 
 					cerr << "Warning: Syntax error in VamPy plugin:  " 
-					<< classname << ". Avoiding plugin." << endl;
+					     << classname << ". Avoiding plugin." << endl;
 					else { 
 							pyPlugs.push_back(pluginKey);
-							m_pyInstances.push_back(pyClassInstance);
+							m_pyClasses.push_back(pyClass);
 						}
 					//pyPlugs.push_back(pluginKey);
 				}
@@ -92,11 +92,11 @@
 }
 
 
-//For now return one class instance found in each script
+//For now return one class object found in each script
 vector<PyObject*> 
-PyPlugScanner::getPyInstances()
+PyPlugScanner::getPyClasses()
 {
-return m_pyInstances;	
+return m_pyClasses;	
 
 }
 
@@ -104,7 +104,7 @@
 //Validate
 //This should not be called more than once!
 PyObject* 
-PyPlugScanner::getScriptInstance(string path, string classname)
+PyPlugScanner::getScriptClass(string path, string classname)
 {
 
 	//Add plugin path to active Python Path 
@@ -136,10 +136,7 @@
 	//Check if class is present and a callable method is implemented
 	if (pyClass && PyCallable_Check(pyClass)) {
 
-		//Create an instance
-		PyObject *pyInstance = PyObject_CallObject(pyClass, NULL);
-		//cerr << "__(getInstance) PyPlugin Class: " << m_class << " successfully created.__" << endl;
-		return pyInstance; 
+	    return pyClass;
 	}	
 	else {
 		cerr << "ERROR: callable plugin class could not be found in source: " << classname << endl 

--- a/PyPlugScanner.h	Thu Jul 16 13:19:20 2009 +0000
+++ b/PyPlugScanner.h	Mon Aug 17 15:22:06 2009 +0000
@@ -54,19 +54,19 @@
 	~PyPlugScanner() { m_hasInstance = false; }
 	static PyPlugScanner *getInstance();	
 	std::vector<std::string> getPyPlugs();
-	std::vector<PyObject*> getPyInstances();
+	std::vector<PyObject*> getPyClasses();
 	void setPath(std::vector<std::string> path);
 	std::vector<std::string> getAllValidPath();
 	
 protected:
 	PyPlugScanner();
-	PyObject *getScriptInstance(std::string path, std::string classname);
+	PyObject *getScriptClass(std::string path, std::string classname);
 	std::vector<std::string> listFiles(std::string dir, std::string ext);
 	static bool m_hasInstance;
 	static PyPlugScanner *m_instance;
 	std::string m_dir;
 	std::vector<std::string> m_path; 
-	std::vector<PyObject*> m_pyInstances;
+	std::vector<PyObject*> m_pyClasses;
 };
 
 #endif	

--- a/PyPlugin.cpp	Thu Jul 16 13:19:20 2009 +0000
+++ b/PyPlugin.cpp	Mon Aug 17 15:22:06 2009 +0000
@@ -75,12 +75,12 @@
 Mutex PyPlugin::m_pythonInterpreterMutex;
 static bool isMapInitialised = false;
 
-PyPlugin::PyPlugin(std::string pluginKey,float inputSampleRate, PyObject *pyInstance) :
-    Plugin(inputSampleRate),
-	m_pyInstance(pyInstance),
+PyPlugin::PyPlugin(std::string pluginKey, float inputSampleRate, PyObject *pyClass) :
+	Plugin(inputSampleRate),
+	m_pyClass(pyClass),
 	m_stepSize(0),
 	m_blockSize(0),
-    m_channels(0),
+	m_channels(0),
 	m_plugin(pluginKey),
 	m_class(pluginKey.substr(pluginKey.rfind(':')+1,pluginKey.size()-1)),
 	m_path((pluginKey.substr(0,pluginKey.rfind(pathsep)))),
@@ -88,14 +88,29 @@
 	m_pyProcess(NULL),
 	m_inputDomain(TimeDomain)
 {	
+	// Create an instance
+	PyObject *pyInputSampleRate = PyFloat_FromDouble(inputSampleRate);
+	PyObject *args = PyTuple_Pack(1, pyInputSampleRate);
+
+	m_pyInstance = PyObject_CallObject(m_pyClass, args);
+
+	if (!m_pyInstance) {
+		cerr << "PyPlugin::PyPlugin: Failed to create Python plugin instance for key \"" << pluginKey << "\" (is the 1-arg class constructor from sample rate correctly provided?)" << endl;
+		throw std::string("Constructor failed");
+	}
+	
+	Py_DECREF(args);
+	Py_DECREF(pyInputSampleRate);
 }
 
 PyPlugin::~PyPlugin()
 {
+	if (m_pyInstance) Py_DECREF(m_pyInstance);
+
 	Py_CLEAR(m_pyProcess);
 #ifdef _DEBUG
 	cerr << "PyPlugin::PyPlugin:" << m_class 
-	<< " Instance deleted." << endl;
+	     << " Instance deleted." << endl;
 #endif
 }
 
@@ -222,7 +237,9 @@
 int
 PyPlugin::getPluginVersion() const
 {
-    return 2;
+	//!!! implement
+
+	return 2;
 }
 
 string
@@ -252,7 +269,8 @@
 		rString=PyString_AsString(pyString);
 		Py_CLEAR(pyString);
 	}
-    return rString;
+
+	return rString;
 }
 
 
@@ -264,8 +282,8 @@
 	cerr << "[call] " << method << endl;
 	
 	//placing Mutex before these calls causes deadlock
-    if (channels < getMinChannelCount() ||
-	channels > getMaxChannelCount()) return false;
+	if (channels < getMinChannelCount() ||
+	    channels > getMaxChannelCount()) return false;
 	
 	m_inputDomain = getInputDomain();
 
@@ -300,45 +318,42 @@
 		m_processType = not_implemented;
 		m_pyProcess = NULL;		
 		cerr << "Warning: Python plugin [" << m_class << "::" << method 
-		<< "] No process implementation found. Plugin will do nothing." << endl;
+		     << "] No process implementation found. Plugin will do nothing." << endl;
 	}
 
-	
-		//Check if the method is implemented in Python else return false
-		if (PyObject_HasAttrString(m_pyInstance,method)) {
+	//Check if the method is implemented in Python else return false
+	if (PyObject_HasAttrString(m_pyInstance,method)) {
    			
-			PyObject *pyMethod = PyString_FromString(method);
-			PyObject *pyChannels = PyInt_FromSsize_t((Py_ssize_t)channels);
-			PyObject *pyStepSize = PyInt_FromSsize_t((Py_ssize_t)m_stepSize);
-			PyObject *pyBlockSize = PyInt_FromSsize_t((Py_ssize_t)blockSize);
-			PyObject *pyInputSampleRate = PyFloat_FromDouble((double)m_inputSampleRate);
-			
-			//Call the method
-			PyObject *pyBool = 
-			PyObject_CallMethodObjArgs(m_pyInstance,pyMethod,pyChannels,pyStepSize,pyBlockSize,pyInputSampleRate,NULL);
+		PyObject *pyMethod = PyString_FromString(method);
+		PyObject *pyChannels = PyInt_FromSsize_t((Py_ssize_t)channels);
+		PyObject *pyStepSize = PyInt_FromSsize_t((Py_ssize_t)m_stepSize);
+		PyObject *pyBlockSize = PyInt_FromSsize_t((Py_ssize_t)blockSize);
+		//Call the method
+		PyObject *pyBool = 
+			PyObject_CallMethodObjArgs(m_pyInstance,pyMethod,pyChannels,pyStepSize,pyBlockSize,NULL);
 						
-			Py_DECREF(pyMethod);
-			Py_DECREF(pyChannels);
-			Py_DECREF(pyStepSize);
-			Py_DECREF(pyBlockSize);
-			Py_DECREF(pyInputSampleRate);
+		Py_DECREF(pyMethod);
+		Py_DECREF(pyChannels);
+		Py_DECREF(pyStepSize);
+		Py_DECREF(pyBlockSize);
 
-			//Check return value
-			if (PyErr_Occurred() || !PyBool_Check(pyBool)) {
-				PyErr_Print(); PyErr_Clear();
-				Py_CLEAR(pyBool);
-				cerr << "ERROR: In Python plugin [" << m_class << "::" << method 
-				<< "] Expected Bool return value." << endl;
-				return false;
-			}
-
-			if (pyBool == Py_True) {  
-				Py_CLEAR(pyBool); 
-				return true;
-			} else {
-				Py_CLEAR(pyBool); 
-				return false;}								
-		} 
+		//Check return value
+		if (PyErr_Occurred() || !PyBool_Check(pyBool)) {
+			PyErr_Print(); PyErr_Clear();
+			Py_CLEAR(pyBool);
+			cerr << "ERROR: In Python plugin [" << m_class << "::" << method 
+			     << "] Expected Bool return value." << endl;
+			return false;
+		}
+		
+		if (pyBool == Py_True) {  
+			Py_CLEAR(pyBool); 
+			return true;
+		} else {
+			Py_CLEAR(pyBool); 
+			return false;
+		}
+	} 
     	return false;
 }
 

--- a/PyPlugin.h	Thu Jul 16 13:19:20 2009 +0000
+++ b/PyPlugin.h	Mon Aug 17 15:22:06 2009 +0000
@@ -108,7 +108,7 @@
 class PyPlugin : public Vamp::Plugin
 {
 public:
-	PyPlugin(std::string plugin,float inputSampleRate, PyObject *pyInstance);
+	PyPlugin(std::string plugin,float inputSampleRate, PyObject *pyClass);
 	virtual ~PyPlugin();
 
 	bool initialise(size_t channels, size_t stepSize, size_t blockSize);
@@ -138,6 +138,7 @@
 	FeatureSet getRemainingFeatures();
 
 protected:
+	PyObject *m_pyClass;
 	PyObject *m_pyInstance;
 	size_t m_stepSize;
 	size_t m_blockSize;

--- a/pyvamp-main.cpp	Thu Jul 16 13:19:20 2009 +0000
+++ b/pyvamp-main.cpp	Mon Aug 17 15:22:06 2009 +0000
@@ -1,44 +1,10 @@
 /* -*- c-basic-offset: 4 indent-tabs-mode: nil -*-  vi:set ts=8 sts=4 sw=4: */
 
-/*
-    Vamp
-
-    An API for audio analysis and feature extraction plugins.
-
-    Centre for Digital Music, Queen Mary, University of London.
-    Copyright 2006 Chris Cannam.
-  
-    Permission is hereby granted, free of charge, to any person
-    obtaining a copy of this software and associated documentation
-    files (the "Software"), to deal in the Software without
-    restriction, including without limitation the rights to use, copy,
-    modify, merge, publish, distribute, sublicense, and/or sell copies
-    of the Software, and to permit persons to whom the Software is
-    furnished to do so, subject to the following conditions:
-
-    The above copyright notice and this permission notice shall be
-    included in all copies or substantial portions of the Software.
-
-    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-    EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
-    MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-    NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
-    ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
-    CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
-    WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-
-    Except as contained in this notice, the names of the Centre for
-    Digital Music; Queen Mary, University of London; and Chris Cannam
-    shall not be used in advertising or otherwise to promote the sale,
-    use or other dealings in this Software without prior written
-    authorization.
-*/
-
 /**
  * This Vamp plugin is a wrapper for Python Scripts. (VamPy)
  * Centre for Digital Music, Queen Mary, University of London.
  * Copyright 2008, George Fazekas.
-*/
+ */
 
 #include <Python.h>
 #include "vamp/vamp.h"
@@ -67,37 +33,37 @@
 class PyPluginAdapter : public Vamp::PluginAdapterBase
 {
 public: 
-	PyPluginAdapter(std::string pyPlugId, PyObject* pyInstance) :
-		PluginAdapterBase(),
-		m_plug(pyPlugId),		
-		m_pyInstance(pyInstance)
-		{ 
-			cerr << "PyPluginAdapter:ctor:"<< adinstcount << ": " << m_plug << endl; 
-			adinstcount++;
-			m_instanceCount = 0;
-		}
-	
-	~PyPluginAdapter() 
-	{
-	}
-
+    PyPluginAdapter(std::string pyPlugId, PyObject* pyClass) :
+        PluginAdapterBase(),
+        m_plug(pyPlugId),		
+        m_pyClass(pyClass)
+    { 
+        cerr << "PyPluginAdapter:ctor:"<< adinstcount << ": " << m_plug << endl; 
+        adinstcount++;
+        m_instanceCount = 0;
+    }
+    
+    ~PyPluginAdapter() 
+    {
+    }
+    
 protected:
-	Vamp::Plugin *createPlugin(float inputSampleRate) {
-        
-		std::string pclass = m_plug.substr(m_plug.rfind(':')+1,m_plug.size()-1);
-		std::string ppath = m_plug.substr(0,m_plug.rfind(pathsep));
-		PyPlugin *plugin = new PyPlugin(m_plug,inputSampleRate,m_pyInstance);
-		m_instanceCount++;
-		cerr << "PyPluginAdapter::createPlugin:" << pclass << " (instance: " << m_instanceCount << ")" << endl;
-		return plugin;
-
-		}
-
-	std::string m_plug;
-	bool m_haveInitialized;
-	PyObject *m_pyInstance;
-	int m_instanceCount;
-
+    Vamp::Plugin *createPlugin(float inputSampleRate)
+    {
+        try {
+            PyPlugin *plugin = new PyPlugin(m_plug, inputSampleRate, m_pyClass);
+            m_instanceCount++;
+            return plugin;
+        } catch (...) {
+            cerr << "PyPluginAdapter::createPlugin: Failed to construct PyPlugin" << endl;
+            return 0;
+        }
+    }
+    
+    std::string m_plug;
+    bool m_haveInitialized;
+    PyObject *m_pyClass;
+    int m_instanceCount;
 };
 
 
@@ -183,8 +149,8 @@
     if (version < 1) return 0;
 
 	int isPythonInitialized = Py_IsInitialized();
-	//cerr << "# isPythonInitialized: " << isPythonInitialized << endl;
-	//cerr << "# haveScannedPlugins: " << haveScannedPlugins << endl;
+	cerr << "# isPythonInitialized: " << isPythonInitialized << endl;
+	cerr << "# haveScannedPlugins: " << haveScannedPlugins << endl;
 
 	if (!haveScannedPlugins) {
 
@@ -210,6 +176,7 @@
 						<< " Dynamic loading in scripts will fail." << endl;
 */
 			Py_Initialize();
+                        cerr << "# isPythonInitialized after initialize: " << Py_IsInitialized() << endl;
 	 		PyEval_InitThreads();			
 		} else {
 			//Py_InitializeEx(1);
@@ -217,7 +184,7 @@
 
 		vector<string> pyPlugs;
 		vector<string> pyPath;
-		vector<PyObject *> pyInstances;
+		vector<PyObject *> pyClasses;
 		static PyPlugScanner *scanner;
 		
 		//Scanning Plugins
@@ -230,11 +197,11 @@
 		pyPlugs = scanner->getPyPlugs();
 		cerr << "Found " << pyPlugs.size() << " Scripts ...OK" << endl;
 		//TODO: this will support multiple classes per script
-		pyInstances = scanner->getPyInstances();
-		cerr << "Found " << pyInstances.size() << " Instances ...OK" << endl;
+		pyClasses = scanner->getPyClasses();
+		cerr << "Found " << pyClasses.size() << " Classes ...OK" << endl;
 
 		for (size_t i = 0; i < pyPlugs.size(); ++i) {
-			adapters.push_back( new PyPluginAdapter(pyPlugs[i],pyInstances[i]));
+			adapters.push_back( new PyPluginAdapter(pyPlugs[i],pyClasses[i]));
 		} 
 		haveScannedPlugins=true;