changeset 197:3b7ec45abd1c

Add mandatory option --json-format to JSON feature writer, in preparation for supporting multiple JSON formats (perhaps) in future
author Chris Cannam
date Tue, 01 Sep 2015 17:05:32 +0100
parents 082c3f21f49e
children 68daabfc4ccc
files .hgsubstate runner/FeatureExtractionManager.cpp runner/JAMSFeatureWriter.cpp runner/JAMSFeatureWriter.h runner/MIDIFeatureWriter.cpp runner/main.cpp tests/test-as-advertised/test-as-advertised.sh tests/test-json-destinations/test-json-destinations.sh tests/test-json-writer/test-json-writer.sh tests/test.sh
diffstat 10 files changed, 114 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- a/.hgsubstate	Tue Sep 01 15:51:07 2015 +0100
+++ b/.hgsubstate	Tue Sep 01 17:05:32 2015 +0100
@@ -1,4 +1,4 @@
 d16f0fd6db6104d87882bc43788a3bb1b0f8c528 dataquay
 55ece8862b6d3a54aad271a53f9c1615e5d3bcf8 sv-dependency-builds
-50210da3997c9e7e1832823d7e3a8e1188d98fc3 svcore
+1cc96e03a903c36f2f9496532fff616c85a22b80 svcore
 632d90c185ecc8655f7a85ba58dc568351449dfd vamp-plugin-sdk
--- a/runner/FeatureExtractionManager.cpp	Tue Sep 01 15:51:07 2015 +0100
+++ b/runner/FeatureExtractionManager.cpp	Tue Sep 01 17:05:32 2015 +0100
@@ -885,7 +885,6 @@
          ti != pi->second.end(); ++ti) {
         
         const Transform &transform = ti->first;
-        const vector<FeatureWriter *> &writers = ti->second;
 
         Transform::SummaryType summaryType = transform.getSummaryType();
         PluginSummarisingAdapter::SummaryType pType =
--- a/runner/JAMSFeatureWriter.cpp	Tue Sep 01 15:51:07 2015 +0100
+++ b/runner/JAMSFeatureWriter.cpp	Tue Sep 01 17:05:32 2015 +0100
@@ -57,10 +57,16 @@
     Parameter p;
 
     p.name = "network";
-    p.description = "Attempt to retrieve RDF descriptions of plugins from network, if not available locally";
+    p.description = "Attempt to retrieve RDF descriptions of plugins from network, if not available locally.";
     p.hasArg = false;
     pl.push_back(p);
 
+    p.name = "format";
+    p.description = "JSON format to use. Currently the only supported value is \"jams\". Other formats may be supported in future.";
+    p.hasArg = true;
+    p.mandatory = true;
+    pl.push_back(p);
+
     return pl;
 }
 
@@ -73,6 +79,13 @@
          i != params.end(); ++i) {
         if (i->first == "network") {
             m_network = true;
+        } else if (i->first == "format") {
+            if (i->second == "jams") {
+                m_format = i->second;
+            } else {
+                throw std::runtime_error
+                    ("Unknown or invalid --json-format parameter \"" + i->second + "\"");
+            }
         }
     }
 }
--- a/runner/JAMSFeatureWriter.h	Tue Sep 01 15:51:07 2015 +0100
+++ b/runner/JAMSFeatureWriter.h	Tue Sep 01 17:05:32 2015 +0100
@@ -95,6 +95,7 @@
 
     QString writeTransformToObjectContents(const Transform &);
 
+    std::string m_format;
     bool m_network;
     bool m_networkRetrieved;
     int m_n;
--- a/runner/MIDIFeatureWriter.cpp	Tue Sep 01 15:51:07 2015 +0100
+++ b/runner/MIDIFeatureWriter.cpp	Tue Sep 01 17:05:32 2015 +0100
@@ -39,7 +39,7 @@
 string
 MIDIFeatureWriter::getDescription() const
 {
-    return "Write features to MIDI files. All features are written as MIDI notes. If a feature has at least one value, its first value will be used as the note pitch, the second value (if present) for velocity. If a feature has units of Hz, then its pitch will be converted from frequency to an integer value in MIDI range, otherwise it will simply be rounded to the nearest integer and written directly. Multiple (up to 16) transforms can be written to a single MIDI file, where they will be given separate MIDI channel numbers.";
+    return "Write features to MIDI files. All features are written as MIDI notes. If a feature has at least one value, its first value will be used as the note pitch, the second value (if present) for velocity. If a feature has units of Hz, then its pitch will be converted from frequency to an integer value in MIDI range; otherwise it will simply be rounded to the nearest integer and written directly. (Be aware that MIDI cannot represent overlapping notes of equal pitch, so features with durations may be misrepresented if they do not have distinct enough values.) Multiple (up to 16) transforms can be written to a single MIDI file, where they will be given separate MIDI channel numbers.";
 }
 
 MIDIFeatureWriter::ParameterList
--- a/runner/main.cpp	Tue Sep 01 15:51:07 2015 +0100
+++ b/runner/main.cpp	Tue Sep 01 17:05:32 2015 +0100
@@ -187,7 +187,7 @@
     cerr << "Sonic Annotator v" << RUNNER_VERSION << endl;
     cerr << "A utility for batch feature extraction from audio files." << endl;
     cerr << "Mark Levy, Chris Sutton and Chris Cannam, Queen Mary, University of London." << endl;
-    cerr << "Copyright 2007-2014 Queen Mary, University of London." << endl;
+    cerr << "Copyright 2007-2015 Queen Mary, University of London." << endl;
     cerr << endl;
     cerr << "This program is free software.  You may redistribute copies of it under the" << endl;
     cerr << "terms of the GNU General Public License <http://www.gnu.org/licenses/gpl.html>." << endl;
@@ -211,6 +211,17 @@
     cerr << endl;
 }
 
+void printOptionHelp(std::string writer, FeatureWriter::Parameter &p)
+{
+    cerr << "  --" << writer << "-" << p.name << " ";
+    int spaceage = 16 - int(writer.length()) - int(p.name.length());
+    if (p.hasArg) { cerr << "<X> "; spaceage -= 4; }
+    for (int k = 0; k < spaceage; ++k) cerr << " ";
+    QString s(p.description.c_str());
+    s = wrap(s, 56, 22);
+    cerr << s << endl;
+}
+
 void printHelp(QString myname, QString w)
 {
     std::string writer = w.toStdString();
@@ -347,23 +358,33 @@
         }
         cerr << "Feature writer \"" << writer << "\":" << endl << endl;
         cerr << "  " << wrap(w->getDescription().c_str(), 76, 2) << endl << endl;
-        cerr << "Additional options for writer type \"" << writer << "\":" << endl;
-        cerr << endl;
         FeatureWriter::ParameterList params = w->getSupportedParameters();
         delete w;
         if (params.empty()) {
             cerr << "  No additional options are available for this writer." << endl << endl;
             return;
         }
-        for (FeatureWriter::ParameterList::const_iterator j = params.begin();
-             j != params.end(); ++j) {
-            cerr << "  --" << writer << "-" << j->name << " ";
-            int spaceage = 16 - int(writer.length()) - int(j->name.length());
-            if (j->hasArg) { cerr << "<X> "; spaceage -= 4; }
-            for (int k = 0; k < spaceage; ++k) cerr << " ";
-            QString s(j->description.c_str());
-            s = wrap(s, 56, 22);
-            cerr << s << endl;
+        FeatureWriter::ParameterList mandatory;
+        bool haveOptional = false;
+        for (auto &p: params) {
+            if (p.mandatory) mandatory.push_back(p);
+            else haveOptional = true;
+        }
+        if (!mandatory.empty()) {
+            cerr << "Mandatory parameters for writer type \"" << writer << "\":" << endl;
+            cerr << endl;
+            for (auto &p: mandatory) {
+                printOptionHelp(writer, p);
+            }
+            cerr << endl;
+        }
+        if (haveOptional) {
+            cerr << "Additional options for writer type \"" << writer << "\":" << endl;
+            cerr << endl;
+            for (auto &p: params) {
+                if (p.mandatory) continue;
+                printOptionHelp(writer, p);
+            }
         }
     }
 
@@ -893,8 +914,35 @@
                 }
             }
         }
-        
-        writer->setParameters(writerArgs);
+
+        for (auto &p: pl) {
+            if (p.mandatory) {
+                bool found = false;
+                for (auto &w: writerArgs) {
+                    if (w.first == p.name) {
+                        found = true;
+                        break;
+                    }
+                }
+                if (!found) {
+                    QString literal = QString("--%1-%2")
+                        .arg(i->c_str()).arg(p.name.c_str());
+                    cerr << myname << ": "
+                         << "the \"" << literal << "\" parameter is mandatory"
+                         << endl;
+                    cerr << helpStr << endl;
+                    exit(2);
+                }
+            }
+        }
+
+        try {
+            writer->setParameters(writerArgs);
+        } catch (std::exception &ex) {
+            cerr << myname << ": " << ex.what() << endl;
+            cerr << helpStr << endl;
+            exit(2);
+        }
         
         writers.push_back(writer);
     }
--- a/tests/test-as-advertised/test-as-advertised.sh	Tue Sep 01 15:51:07 2015 +0100
+++ b/tests/test-as-advertised/test-as-advertised.sh	Tue Sep 01 17:05:32 2015 +0100
@@ -31,6 +31,10 @@
     # that have no values (but are only point events).  I don't know
     # how reasonable that is, but it's clearly intentional.  It also
     # writes to a subdirectory $basedir/$catid/$trackid.$output
+    #
+    # * The "json" reader has a mandatory --json-format parameter that
+    # currently only accepts one argument ("jams"). It should fail if
+    # run with any other value or without this parameter.
 
     case $type in
 	audiodb) 
@@ -42,6 +46,16 @@
 	    $r -t $onsets -w $type $tmpwav > $tmpdir/test.out 2>/dev/null || \
 		fail "Fails to run with reader type \"$type\" and default options"
 	    ;;
+	json)
+	    $r -t $onsets -w $type $tmpwav 2>/dev/null && \
+		fail "Wrongly succeeds in running with reader type \"$type\" and default options"
+	    $r -t $onsets -w $type $tmpwav --json-format blah 2>/dev/null && \
+		fail "Wrongly succeeds in running with reader type \"$type\" and unknown json-format option"
+	    $r -t $onsets -w $type $tmpwav --json-format 2>/dev/null && \
+		fail "Wrongly succeeds in running with reader type \"$type\" and empty json-format option"
+	    $r -t $onsets -w $type $tmpwav --json-format jams 2>/dev/null || \
+		fail "Fails to run with reader type \"$type\" and correct json-format option"
+	    ;;
 	*)
 	    $r -t $onsets -w $type $tmpwav 2>/dev/null || \
 		fail "Fails to run with reader type \"$type\" and default options"
--- a/tests/test-json-destinations/test-json-destinations.sh	Tue Sep 01 15:51:07 2015 +0100
+++ b/tests/test-json-destinations/test-json-destinations.sh	Tue Sep 01 17:05:32 2015 +0100
@@ -23,12 +23,14 @@
 
 transformdir=$mypath/transforms
 
+mandatory="-w json --json-format jams"
+
 
 ctx="onsets transform, one audio file, default JSON writer destination"
 
 rm -f $audiopath/$outfile1
 
-$r -t $transformdir/onsets.n3 -w json $infile1 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory $infile1 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile1 "$ctx"
@@ -40,7 +42,7 @@
 
 cp $infile1 $infile1dot
 
-$r -t $transformdir/onsets.n3 -w json $infile1dot 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory $infile1dot 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile1dot "$ctx"
@@ -52,7 +54,7 @@
 
 rm -f $audiopath/$outfile1
 
-$r -t $transformdir/onsets.n3 -t $transformdir/detectionfunction.n3 -w json $infile1 2>/dev/null || \
+$r -t $transformdir/onsets.n3 -t $transformdir/detectionfunction.n3 $mandatory $infile1 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile1 "$ctx"
@@ -63,7 +65,7 @@
 rm -f $audiopath/$outfile1
 rm -f $audiopath/$outfile2
 
-$r -t $transformdir/onsets.n3 -w json $infile1 $infile2 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory $infile1 $infile2 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile1 "$ctx"
@@ -72,7 +74,7 @@
 
 ctx="onsets transform, two audio files, one-file JSON writer"
 
-$r -t $transformdir/onsets.n3 -w json --json-one-file $tmpjson $infile1 $infile2 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory --json-one-file $tmpjson $infile1 $infile2 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $tmpjson "$ctx"
@@ -80,7 +82,7 @@
 
 ctx="onsets transform, two audio files, stdout JSON writer"
 
-$r -t $transformdir/onsets.n3 -w json --json-stdout $infile1 $infile2 2>/dev/null >$tmpjson || \
+$r -t $transformdir/onsets.n3 $mandatory --json-stdout $infile1 $infile2 2>/dev/null >$tmpjson || \
     fail "Fails to run with $ctx"
 
 check_json $tmpjson "$ctx"
@@ -90,7 +92,7 @@
 
 rm -f $audiopath/$outfile3
 
-$r -t $transformdir/onsets.n3 -w json --json-many-files $infile1 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory --json-many-files $infile1 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile3 "$ctx"
@@ -101,7 +103,7 @@
 rm -f $audiopath/$outfile3
 rm -f $audiopath/$outfile5
 
-$r -t $transformdir/onsets.n3 -w json --json-many-files $infile1 $infile2 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory --json-many-files $infile1 $infile2 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile3 "$ctx"
@@ -115,7 +117,7 @@
 rm -f $audiopath/$outfile5
 rm -f $audiopath/$outfile6
 
-$r -t $transformdir/onsets.n3 -t $transformdir/detectionfunction.n3 -w json --json-many-files $infile1 $infile2 2>/dev/null || \
+$r -t $transformdir/onsets.n3 -t $transformdir/detectionfunction.n3 $mandatory --json-many-files $infile1 $infile2 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile3 "$ctx"
@@ -128,7 +130,7 @@
 
 rm -f ./$outfile1
 
-$r -t $transformdir/onsets.n3 -t $transformdir/detectionfunction.n3 -w json --json-basedir . $infile1 2>/dev/null || \
+$r -t $transformdir/onsets.n3 -t $transformdir/detectionfunction.n3 $mandatory --json-basedir . $infile1 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json ./$outfile1 "$ctx"
@@ -139,7 +141,7 @@
 rm -f ./$outfile3
 rm -f ./$outfile5
 
-$r -t $transformdir/onsets.n3 -w json --json-basedir . --json-many-files $infile1 $infile2 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory --json-basedir . --json-many-files $infile1 $infile2 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json ./$outfile3 "$ctx"
@@ -148,7 +150,7 @@
 
 ctx="nonexistent output base directory"
 
-$r -t $transformdir/onsets.n3 -w json --json-basedir ./DOES_NOT_EXIST $infile1 2>/dev/null && \
+$r -t $transformdir/onsets.n3 $mandatory --json-basedir ./DOES_NOT_EXIST $infile1 2>/dev/null && \
     fail "Fails with $ctx by completing successfully (should refuse and bail out)"
 
 
@@ -156,7 +158,7 @@
 
 touch $audiopath/$outfile1
 
-$r -t $transformdir/onsets.n3 -w json $infile1 2>/dev/null && \
+$r -t $transformdir/onsets.n3 $mandatory $infile1 2>/dev/null && \
     fail "Fails by completing successfully when output file already exists (should refuse and bail out)"
 
 
@@ -164,7 +166,7 @@
 
 touch $audiopath/$outfile1
 
-$r -t $transformdir/onsets.n3 -w json --json-force $infile1 2>/dev/null || \
+$r -t $transformdir/onsets.n3 $mandatory --json-force $infile1 2>/dev/null || \
     fail "Fails to run with $ctx"
 
 check_json $audiopath/$outfile1 "$ctx"
--- a/tests/test-json-writer/test-json-writer.sh	Tue Sep 01 15:51:07 2015 +0100
+++ b/tests/test-json-writer/test-json-writer.sh	Tue Sep 01 17:05:32 2015 +0100
@@ -10,11 +10,13 @@
 
 transformdir=$mypath/transforms
 
+mandatory="-w json --json-format jams"
+
 # This does not yet test for correct values, only for parseable json
 
 for output in instants curve-oss curve-fsr curve-fsr-timed curve-vsr grid-oss grid-fsr notes-regions; do
 
-    $r -d "$testplug:$output" -w json --json-one-file "$tmpjson" --json-force "$silentfile" 2>/dev/null || \
+    $r -d "$testplug:$output" $mandatory --json-one-file "$tmpjson" --json-force "$silentfile" 2>/dev/null || \
 	fail "Failed to run for plugin $testplug with output $output"
 
     check_json "$tmpjson" "test plugin output $output"
--- a/tests/test.sh	Tue Sep 01 15:51:07 2015 +0100
+++ b/tests/test.sh	Tue Sep 01 17:05:32 2015 +0100
@@ -15,10 +15,10 @@
     csv-destinations \
     lab-writer \
     lab-destinations \
+    midi-writer \
     midi-destinations \
-    midi-writer \
+    json-writer \
     json-destinations \
-    json-writer \
     summaries \
     multiple-audio \
     ; do