changeset 52:e90fd30990eb

Error handling, and pass plugin handles through
author Chris Cannam <c.cannam@qmul.ac.uk>
date Fri, 16 Sep 2016 16:20:05 +0100
parents f4244a2d55ac
children 9bebe2780cc0
files bits/CountingPluginHandleMapper.h bits/PreservingPluginHandleMapper.h capnproto/VampnProto.h capnproto/vamp.capnp json/VampJson.h test/test-vampipe-server.sh utilities/vampipe-convert.cpp utilities/vampipe-server.cpp
diffstat 8 files changed, 209 insertions(+), 94 deletions(-) [+]
line wrap: on
line diff
--- a/bits/CountingPluginHandleMapper.h	Fri Sep 16 15:10:57 2016 +0100
+++ b/bits/CountingPluginHandleMapper.h	Fri Sep 16 16:20:05 2016 +0100
@@ -67,6 +67,7 @@
 
     void removePlugin(Handle h) {
 	if (m_plugins.find(h) == m_plugins.end()) {
+            std::cerr << "remove: no handle " << h << std::endl;
 	    throw NotFound();
 	}
 	Vamp::Plugin *p = m_plugins[h];
@@ -82,6 +83,7 @@
     
     Handle pluginToHandle(Vamp::Plugin *p) const {
 	if (m_rplugins.find(p) == m_rplugins.end()) {
+            std::cerr << "pluginToHandle: no plugin " << p << std::endl;
 	    throw NotFound();
 	}
 	return m_rplugins.at(p);
@@ -89,6 +91,7 @@
     
     Vamp::Plugin *handleToPlugin(Handle h) const {
 	if (m_plugins.find(h) == m_plugins.end()) {
+            std::cerr << "handleToPlugin: no handle " << h << std::endl;
 	    throw NotFound();
 	}
 	return m_plugins.at(h);
@@ -97,11 +100,13 @@
     //!!! iffy: mapper will move when another plugin is added. return by value?
     const PluginOutputIdMapper &pluginToOutputIdMapper(Vamp::Plugin *p) const {
         // pluginToHandle checks the plugin has been registered with us
+        std::cerr << "output id mapper requested for plugin with handle " << pluginToHandle(p) << std::endl;
         return *m_outputMappers.at(pluginToHandle(p));
     }
 
     //!!! iffy: mapper will move when another plugin is added. return by value?
     const PluginOutputIdMapper &handleToOutputIdMapper(Handle h) const {
+        std::cerr << "output id mapper requested for handle " << h << std::endl;
 	if (m_plugins.find(h) == m_plugins.end()) {
 	    throw NotFound();
 	}
--- a/bits/PreservingPluginHandleMapper.h	Fri Sep 16 15:10:57 2016 +0100
+++ b/bits/PreservingPluginHandleMapper.h	Fri Sep 16 16:20:05 2016 +0100
@@ -51,7 +51,7 @@
 public:
     PreservingPluginHandleMapper() : m_handle(0), m_plugin(0) { }
 
-    virtual int32_t pluginToHandle(Vamp::Plugin *p) const {
+    virtual Handle pluginToHandle(Vamp::Plugin *p) const {
 	if (p == m_plugin) return m_handle;
 	else {
 	    std::cerr << "PreservingPluginHandleMapper: p = " << p
@@ -62,7 +62,7 @@
 	}
     }
 
-    virtual Vamp::Plugin *handleToPlugin(int32_t h) const {
+    virtual Vamp::Plugin *handleToPlugin(Handle h) const {
 	m_handle = h;
 	m_plugin = reinterpret_cast<Vamp::Plugin *>(h);
 	return m_plugin;
@@ -72,12 +72,12 @@
         return m_omapper;
     }
         
-    virtual const PluginOutputIdMapper &handleToOutputIdMapper(int32_t h) const {
+    virtual const PluginOutputIdMapper &handleToOutputIdMapper(Handle h) const {
         return m_omapper;
     }
     
 private:
-    mutable int32_t m_handle;
+    mutable Handle m_handle;
     mutable Vamp::Plugin *m_plugin;
     PreservingPluginOutputIdMapper m_omapper;
 };
--- a/capnproto/VampnProto.h	Fri Sep 16 15:10:57 2016 +0100
+++ b/capnproto/VampnProto.h	Fri Sep 16 16:20:05 2016 +0100
@@ -654,6 +654,7 @@
                          const Vamp::HostExt::ProcessResponse &pr,
                          const PluginHandleMapper &pmapper) {
 
+        b.setPluginHandle(pmapper.pluginToHandle(pr.plugin));
         auto f = b.initFeatures();
         buildFeatureSet(f, pr.features,
                         pmapper.pluginToOutputIdMapper(pr.plugin));
@@ -664,6 +665,8 @@
                         const ProcessResponse::Reader &r,
                         const PluginHandleMapper &pmapper) {
 
+        auto h = r.getPluginHandle();
+        pr.plugin = pmapper.handleToPlugin(h);
         readFeatureSet(pr.features, r.getFeatures(),
                        pmapper.handleToOutputIdMapper(r.getPluginHandle()));
     }
@@ -756,6 +759,44 @@
         buildVampResponse_Process(b, pr, pmapper);
     }
 
+    static void
+    buildVampResponse_Error(VampResponse::Builder &b,
+                            const std::string &errorText,
+                            RRType responseType)
+    {
+        std::string type;
+
+        if (responseType == RRType::List) {
+            type = "list";
+            b.getResponse().initList();
+        } else if (responseType == RRType::Load) {
+            type = "load";
+            b.getResponse().initLoad();
+        } else if (responseType == RRType::Configure) {
+            type = "configure";
+            b.getResponse().initConfigure();
+        } else if (responseType == RRType::Process) {
+            type = "process";
+            b.getResponse().initProcess();
+        } else if (responseType == RRType::Finish) {
+            type = "finish";
+            b.getResponse().initFinish();
+        } else {
+            type = "invalid";
+        }
+
+        b.setSuccess(false);
+        b.setErrorText(std::string("error in ") + type + " request: " + errorText);
+    }
+
+    static void
+    buildVampResponse_Exception(VampResponse::Builder &b,
+                                const std::exception &e,
+                                RRType responseType)
+    {
+        return buildVampResponse_Error(b, e.what(), responseType);
+    }
+    
     static RRType
     getRequestResponseType(const VampRequest::Reader &r) {
         switch (r.getRequest().which()) {
--- a/capnproto/vamp.capnp	Fri Sep 16 15:10:57 2016 +0100
+++ b/capnproto/vamp.capnp	Fri Sep 16 16:20:05 2016 +0100
@@ -136,6 +136,7 @@
 }
 
 struct ConfigurationResponse {
+#!!! now the only response type not to have the pluginHandle, so maybe it should, just for completeness
     outputs            @0  :List(OutputDescriptor);
 }
 
--- a/json/VampJson.h	Fri Sep 16 15:10:57 2016 +0100
+++ b/json/VampJson.h	Fri Sep 16 16:20:05 2016 +0100
@@ -1001,9 +1001,12 @@
         jo["type"] = "process";
         jo["success"] = true;
         jo["errorText"] = "";
-        jo["content"] = fromFeatureSet(resp.features,
-                                       pmapper.pluginToOutputIdMapper(resp.plugin),
-                                       serialisation);
+        json11::Json::object po;
+        po["pluginHandle"] = pmapper.pluginToHandle(resp.plugin);
+        po["features"] = fromFeatureSet(resp.features,
+                                        pmapper.pluginToOutputIdMapper(resp.plugin),
+                                        serialisation);
+        jo["content"] = po;
         return json11::Json(jo);
     }
     
@@ -1028,14 +1031,17 @@
         jo["type"] = "finish";
         jo["success"] = true;
         jo["errorText"] = "";
-        jo["content"] = fromFeatureSet(resp.features,
-                                       pmapper.pluginToOutputIdMapper(resp.plugin),
-                                       serialisation);
+        json11::Json::object po;
+        po["pluginHandle"] = pmapper.pluginToHandle(resp.plugin);
+        po["features"] = fromFeatureSet(resp.features,
+                                        pmapper.pluginToOutputIdMapper(resp.plugin),
+                                        serialisation);
+        jo["content"] = po;
         return json11::Json(jo);
     }
 
     static json11::Json
-    fromException(const std::exception &e, RRType responseType) {
+    fromError(std::string errorText, RRType responseType) {
 
         json11::Json::object jo;
         std::string type;
@@ -1049,10 +1055,15 @@
 
         jo["type"] = type;
         jo["success"] = false;
-        jo["errorText"] = std::string("exception caught: ") +
-            type + " request: " + e.what();
+        jo["errorText"] = std::string("error in ") + type + " request: " + errorText;
         return json11::Json(jo);
     }
+
+    static json11::Json
+    fromException(const std::exception &e, RRType responseType) {
+
+        return fromError(e.what(), responseType);
+    }
     
 private: // go private briefly for a couple of helper functions
     
@@ -1163,10 +1174,11 @@
         Vamp::HostExt::ProcessResponse resp;
         if (successful(j)) {
             auto jc = j["content"];
-            resp.features = toFeatureSet
-                (jc["features"],
-                 pmapper.handleToOutputIdMapper(jc["pluginHandle"].int_value()),
-                 serialisation);
+            auto h = jc["pluginHandle"].int_value();
+            resp.plugin = pmapper.handleToPlugin(h);
+            resp.features = toFeatureSet(jc["features"],
+                                         pmapper.handleToOutputIdMapper(h),
+                                         serialisation);
         }
         return resp;
     }
@@ -1186,10 +1198,11 @@
         Vamp::HostExt::ProcessResponse resp;
         if (successful(j)) {
             auto jc = j["content"];
-            resp.features = toFeatureSet
-                (jc["features"],
-                 pmapper.handleToOutputIdMapper(jc["pluginHandle"].int_value()),
-                 serialisation);
+            auto h = jc["pluginHandle"].int_value();
+            resp.plugin = pmapper.handleToPlugin(h);
+            resp.features = toFeatureSet(jc["features"],
+                                         pmapper.handleToOutputIdMapper(h),
+                                         serialisation);
         }
         return resp;
     }
--- a/test/test-vampipe-server.sh	Fri Sep 16 15:10:57 2016 +0100
+++ b/test/test-vampipe-server.sh	Fri Sep 16 16:20:05 2016 +0100
@@ -2,7 +2,11 @@
 
 ( bin/vampipe-convert request -i json -o capnp |
 	VAMP_PATH=./vamp-plugin-sdk/examples bin/vampipe-server |
-	bin/vampipe-convert response -i capnp -o json ) <<EOF
+
+#	capnp decode capnproto/vamp.capnp VampResponse
+
+	bin/vampipe-convert response -i capnp -o json
+) <<EOF
 {"type":"list"}
 {"type":"load","content": {"pluginKey":"vamp-example-plugins:percussiononsets","inputSampleRate":44100,"adapterFlags":["AdaptInputDomain","AdaptBufferSize"]}}
 {"type":"configure","content":{"pluginHandle":1,"configuration":{"blockSize": 8, "channelCount": 1, "parameterValues": {"sensitivity": 40, "threshold": 3}, "stepSize": 8}}}
--- a/utilities/vampipe-convert.cpp	Fri Sep 16 15:10:57 2016 +0100
+++ b/utilities/vampipe-convert.cpp	Fri Sep 16 16:20:05 2016 +0100
@@ -172,6 +172,9 @@
     rr.type = VampJson::getRequestResponseType(j);
     VampJson::BufferSerialisation serialisation = VampJson::BufferSerialisation::Text;
 
+    rr.success = j["success"].bool_value();
+    rr.errorText = j["errorText"].string_value();
+
     switch (rr.type) {
 
     case RRType::List:
@@ -201,37 +204,44 @@
 {
     Json j;
 
-    switch (rr.type) {
+    if (!rr.success) {
 
-    case RRType::List:
-	j = VampJson::fromVampResponse_List("", rr.listResponse);
-	break;
-    case RRType::Load:
-	j = VampJson::fromVampResponse_Load(rr.loadResponse, mapper);
-	break;
-    case RRType::Configure:
-	j = VampJson::fromVampResponse_Configure(rr.configurationResponse);
-	break;
-    case RRType::Process:
-	j = VampJson::fromVampResponse_Process
-	    (rr.processResponse,
-	     mapper,
-	     useBase64 ?
-	     VampJson::BufferSerialisation::Base64 :
-	     VampJson::BufferSerialisation::Text);
-	break;
-    case RRType::Finish:
-	j = VampJson::fromVampResponse_Finish
-	    (rr.finishResponse,
-	     mapper,
-	     useBase64 ?
-	     VampJson::BufferSerialisation::Base64 :
-	     VampJson::BufferSerialisation::Text);
-	break;
-    case RRType::NotValid:
-	break;
+	j = VampJson::fromError(rr.errorText, rr.type);
+
+    } else {
+    
+	switch (rr.type) {
+
+	case RRType::List:
+	    j = VampJson::fromVampResponse_List("", rr.listResponse);
+	    break;
+	case RRType::Load:
+	    j = VampJson::fromVampResponse_Load(rr.loadResponse, mapper);
+	    break;
+	case RRType::Configure:
+	    j = VampJson::fromVampResponse_Configure(rr.configurationResponse);
+	    break;
+	case RRType::Process:
+	    j = VampJson::fromVampResponse_Process
+		(rr.processResponse,
+		 mapper,
+		 useBase64 ?
+		 VampJson::BufferSerialisation::Base64 :
+		 VampJson::BufferSerialisation::Text);
+	    break;
+	case RRType::Finish:
+	    j = VampJson::fromVampResponse_Finish
+		(rr.finishResponse,
+		 mapper,
+		 useBase64 ?
+		 VampJson::BufferSerialisation::Base64 :
+		 VampJson::BufferSerialisation::Text);
+	    break;
+	case RRType::NotValid:
+	    break;
+	}
     }
-
+    
     cout << j.dump() << endl;
 }
 
@@ -312,6 +322,8 @@
     VampResponse::Reader reader = message.getRoot<VampResponse>();
     
     rr.type = VampnProto::getRequestResponseType(reader);
+    rr.success = reader.getSuccess();
+    rr.errorText = reader.getErrorText();
 
     switch (rr.type) {
 
@@ -344,27 +356,34 @@
     ::capnp::MallocMessageBuilder message;
     VampResponse::Builder builder = message.initRoot<VampResponse>();
 
-    switch (rr.type) {
+    if (!rr.success) {
 
-    case RRType::List:
-	VampnProto::buildVampResponse_List(builder, "", rr.listResponse);
-	break;
-    case RRType::Load:
-	VampnProto::buildVampResponse_Load(builder, rr.loadResponse, mapper);
-	break;
-    case RRType::Configure:
-	VampnProto::buildVampResponse_Configure(builder, rr.configurationResponse);
-	break;
-    case RRType::Process:
-	VampnProto::buildVampResponse_Process(builder, rr.processResponse, mapper);
-	break;
-    case RRType::Finish:
-	VampnProto::buildVampResponse_Finish(builder, rr.finishResponse, mapper);
-	break;
-    case RRType::NotValid:
-	break;
+	VampnProto::buildVampResponse_Error(builder, rr.errorText, rr.type);
+
+    } else {
+	
+	switch (rr.type) {
+
+	case RRType::List:
+	    VampnProto::buildVampResponse_List(builder, "", rr.listResponse);
+	    break;
+	case RRType::Load:
+	    VampnProto::buildVampResponse_Load(builder, rr.loadResponse, mapper);
+	    break;
+	case RRType::Configure:
+	    VampnProto::buildVampResponse_Configure(builder, rr.configurationResponse);
+	    break;
+	case RRType::Process:
+	    VampnProto::buildVampResponse_Process(builder, rr.processResponse, mapper);
+	    break;
+	case RRType::Finish:
+	    VampnProto::buildVampResponse_Finish(builder, rr.finishResponse, mapper);
+	    break;
+	case RRType::NotValid:
+	    break;
+	}
     }
-
+    
     writeMessageToFd(1, message);
 }
 
@@ -485,6 +504,7 @@
 	    writeOutput(outformat, rr);
 	    
 	} catch (std::exception &e) {
+
 	    cerr << "Error: " << e.what() << endl;
 	    exit(1);
 	}
--- a/utilities/vampipe-server.cpp	Fri Sep 16 15:10:57 2016 +0100
+++ b/utilities/vampipe-server.cpp	Fri Sep 16 16:20:05 2016 +0100
@@ -80,27 +80,44 @@
     ::capnp::MallocMessageBuilder message;
     VampResponse::Builder builder = message.initRoot<VampResponse>();
 
-    switch (rr.type) {
+    if (!rr.success) {
 
-    case RRType::List:
-	VampnProto::buildVampResponse_List(builder, "", rr.listResponse);
-	break;
-    case RRType::Load:
-	VampnProto::buildVampResponse_Load(builder, rr.loadResponse, mapper);
-	break;
-    case RRType::Configure:
-	VampnProto::buildVampResponse_Configure(builder, rr.configurationResponse);
-	break;
-    case RRType::Process:
-	VampnProto::buildVampResponse_Process(builder, rr.processResponse, mapper);
-	break;
-    case RRType::Finish:
-	VampnProto::buildVampResponse_Finish(builder, rr.finishResponse, mapper);
-	break;
-    case RRType::NotValid:
-	break;
+	VampnProto::buildVampResponse_Error(builder, rr.errorText, rr.type);
+
+    } else {
+	
+	switch (rr.type) {
+
+	case RRType::List:
+	    VampnProto::buildVampResponse_List(builder, "", rr.listResponse);
+	    break;
+	case RRType::Load:
+	    VampnProto::buildVampResponse_Load(builder, rr.loadResponse, mapper);
+	    break;
+	case RRType::Configure:
+	    VampnProto::buildVampResponse_Configure(builder, rr.configurationResponse);
+	    break;
+	case RRType::Process:
+	    VampnProto::buildVampResponse_Process(builder, rr.processResponse, mapper);
+	    break;
+	case RRType::Finish:
+	    VampnProto::buildVampResponse_Finish(builder, rr.finishResponse, mapper);
+	    break;
+	case RRType::NotValid:
+	    break;
+	}
     }
+    
+    writeMessageToFd(1, message);
+}
 
+void
+writeExceptionCapnp(const std::exception &e, RRType type)
+{
+    ::capnp::MallocMessageBuilder message;
+    VampResponse::Builder builder = message.initRoot<VampResponse>();
+    VampnProto::buildVampResponse_Exception(builder, e, type);
+    
     writeMessageToFd(1, message);
 }
 
@@ -168,6 +185,7 @@
 	    fbuffers[i] = preq.inputBuffers[i].data();
 	}
 
+	response.processResponse.plugin = preq.plugin;
 	response.processResponse.features =
 	    preq.plugin->process(fbuffers, preq.timestamp);
 	response.success = true;
@@ -178,13 +196,14 @@
 
     case RRType::Finish:
     {
-	auto h = mapper.pluginToHandle(request.finishPlugin);
-
+	response.finishResponse.plugin = request.finishPlugin;
 	response.finishResponse.features =
 	    request.finishPlugin->getRemainingFeatures();
-	    
-	mapper.removePlugin(h);
-	delete request.finishPlugin;
+
+	// We do not delete the plugin here -- we need it in the
+	// mapper when converting the features. It gets deleted by the
+	// caller.
+	
 	response.success = true;
 	break;
     }
@@ -204,9 +223,11 @@
 
     while (true) {
 
+	RequestOrResponse request;
+	
 	try {
 
-	    RequestOrResponse request = readRequestCapnp();
+	    request = readRequestCapnp();
 
 	    cerr << "vampipe-server: request received, of type "
 		 << int(request.type)
@@ -226,9 +247,19 @@
 	    writeResponseCapnp(response);
 
 	    cerr << "vampipe-server: response written" << endl;
+
+	    if (request.type == RRType::Finish) {
+		auto h = mapper.pluginToHandle(request.finishPlugin);
+		mapper.removePlugin(h);
+		delete request.finishPlugin;
+	    }
 	    
 	} catch (std::exception &e) {
+
 	    cerr << "vampipe-server: error: " << e.what() << endl;
+
+	    writeExceptionCapnp(e, request.type);
+	    
 	    exit(1);
 	}
     }