diff --git a/.github/workflows/production-deploy.yml b/.github/workflows/production-deploy.yml index 6229778..cdc1ac1 100644 --- a/.github/workflows/production-deploy.yml +++ b/.github/workflows/production-deploy.yml @@ -52,7 +52,8 @@ jobs: secrets: aws-key-id: ${{ secrets.LAMBDA_CONTAINER_PIPELINE_AWS_ID }} aws-secret-key: ${{ secrets.LAMBDA_CONTAINER_PIPELINE_AWS_SECRET}} - function-admin-api-key: ${{ secrets.FUNCTION_ADMIN_API_KEY}} + function-admin-api-key-staging: ${{ secrets.FUNCTION_ADMIN_API_KEY_STAGING }} + function-admin-api-key-production: ${{ secrets.FUNCTION_ADMIN_API_KEY_PRODUCTION }} gcp_credentials: ${{ secrets.GCP_DEPLOY }} TEST_API_ENDPOINT: ${{ secrets.TEST_API_ENDPOINT }} DB_USER: ${{ secrets.DB_USER }} diff --git a/.github/workflows/staging-deploy.yml b/.github/workflows/staging-deploy.yml index f1f4b88..93fb75f 100644 --- a/.github/workflows/staging-deploy.yml +++ b/.github/workflows/staging-deploy.yml @@ -65,5 +65,5 @@ jobs: secrets: aws-key-id: ${{ secrets.LAMBDA_CONTAINER_PIPELINE_AWS_ID }} aws-secret-key: ${{ secrets.LAMBDA_CONTAINER_PIPELINE_AWS_SECRET}} - function-admin-api-key: ${{ secrets.FUNCTION_ADMIN_API_KEY}} + function-admin-api-key-staging: ${{ secrets.FUNCTION_ADMIN_API_KEY_STAGING }} gcp_credentials: ${{ secrets.GCP_DEPLOY }} diff --git a/Dockerfile b/Dockerfile index 46a7efb..fbfbdb2 100755 --- a/Dockerfile +++ b/Dockerfile @@ -25,9 +25,20 @@ RUN /app/.venv/bin/pip uninstall -y \ FROM ghcr.io/lambda-feedback/evaluation-function-base/python:3.11 +# librosa decodes wav/flac/ogg/opus/mp3 itself through libsndfile, but m4a and +# aac fall back to audioread, which shells out to ffmpeg. +RUN apt-get update \ + && apt-get install -y --no-install-recommends ffmpeg \ + && rm -rf /var/lib/apt/lists/* + ENV VIRTUAL_ENV=/app/.venv \ PATH="/app/.venv/bin:$PATH" +# Lambda's filesystem is read-only outside /tmp. librosa's numba functions +# cache their compiled code next to the source by default, which fails there +# ("cannot cache function ... no locator available"), so cache in /tmp. +ENV NUMBA_CACHE_DIR=/tmp/numba_cache + COPY --from=builder ${VIRTUAL_ENV} ${VIRTUAL_ENV} # Precompile python files for faster startup diff --git a/README.md b/README.md index 64f903b..c06302e 100755 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ An evaluation function for the [Lambda Feedback](https://lambdafeedback.com) platform. It compares a student's music performance against a reference performance and returns formative feedback on pitch accuracy, timing, note duration and chords. -The student's response and the reference answer can each be MIDI note data, or the path to an audio recording, which is transcribed to MIDI before the comparison. +The student's response and the reference answer can each be MIDI note data, a path or URL to a MIDI file (`.mid` / `.midi`), or a path or URL to an audio recording (`.mp3`, `.wav`, `.ogg`, `.aac`, `.flac`, `.opus` or `.m4a`), which is transcribed to MIDI before the comparison. ## Deployment diff --git a/docs/dev.md b/docs/dev.md index b021d56..912cb20 100644 --- a/docs/dev.md +++ b/docs/dev.md @@ -12,7 +12,7 @@ Automated formative feedback on music practice. Compares a student's performance | `global_fast_threshold` | `0.85` | Overall tempo more than 15% faster than reference triggers a "too fast" comment. | | `chord_onset_window` | `0.05` | How close together (in seconds) notes need to start to be treated as one chord rather than separate notes. Increase this if fast chords are being incorrectly split into separate notes; decrease it if separate fast notes are being incorrectly grouped into a chord. | -Both `response` and `answer` (i.e. reference) can be audio file path of music recordings, or MIDI (a dict, or a JSON string of a dict), they will be converted to the following format to pass through the pipeline: +Both `response` and `answer` (i.e. reference) can be a path or URL to an audio recording (`.mp3`, `.wav`, `.ogg`, `.aac`, `.flac`, `.opus` or `.m4a`, transcribed to MIDI first), a path or URL to a MIDI file (`.mid` / `.midi`), or MIDI (a dict, or a JSON string of a dict), they will be converted to the following format to pass through the pipeline: ```json { "notes": [ diff --git a/docs/user.md b/docs/user.md index 736e2ec..75e1678 100644 --- a/docs/user.md +++ b/docs/user.md @@ -12,7 +12,7 @@ The feedback message was structured into four parts. ## Setting up a question -The **Answer** and **Response** can be audio file path of music recordings, or MIDI (a dict, or a JSON string of a dict) format such as: +The **Answer** and **Response** can be a path or URL to an audio recording (`.mp3`, `.wav`, `.ogg`, `.aac`, `.flac`, `.opus` or `.m4a`, transcribed to MIDI first), a path or URL to a MIDI file (`.mid` / `.midi`), or MIDI (a dict, or a JSON string of a dict) format such as: ```json { "notes": [ diff --git a/evaluation_function/audio_processing.py b/evaluation_function/audio_processing.py index e3e382f..b09b5f9 100644 --- a/evaluation_function/audio_processing.py +++ b/evaluation_function/audio_processing.py @@ -16,6 +16,10 @@ import contextlib import io import os +import shutil +import tempfile +import urllib.request +from urllib.parse import urlparse # basic_pitch (and its librosa / numba / onnxruntime dependencies) is a very # heavy import - tens of seconds on a cold Lambda. It is imported lazily inside @@ -41,11 +45,108 @@ MAX_FRAGMENT_DURATION = 0.15 -# File extensions we treat as "audio that needs transcription" -AUDIO_EXTENSIONS = [".wav", ".mp3", ".m4a", ".flac", ".ogg"] +# File extensions we treat as "audio that needs transcription". +# librosa decodes wav, flac, ogg, opus and mp3 through libsndfile; m4a and aac +# fall back to audioread, which needs the ffmpeg installed in the Dockerfile. +AUDIO_EXTENSIONS = [".wav", ".mp3", ".m4a", ".aac", ".flac", ".ogg", ".opus"] # File extensions we treat as "already MIDI, no transcription needed" MIDI_EXTENSIONS = [".mid", ".midi"] +# MIME types the platform's Audio response area reports for uploaded files, +# used when neither the URL nor the file name carries a known extension. +# Mirrors AUDIO_MIME_TYPES in client-web's Audio.schema.ts. +MIME_TYPE_EXTENSIONS = { + "audio/mpeg": ".mp3", + "audio/wav": ".wav", + "audio/mp4": ".m4a", + "audio/ogg": ".ogg", + "audio/aac": ".aac", + "audio/flac": ".flac", + "audio/opus": ".opus", + "audio/midi": ".mid", +} + +# How long to wait for an audio download before giving up. +DOWNLOAD_TIMEOUT_SECONDS = 30 + + +# Helpers for reading a local path or a URL +# --------------------------------------------------------------------- +def file_extension(path): + """ + Return the lowercase file extension of a local path or a URL. + + Audio submitted from the platform arrives as a URL to an object in a + bucket, and those URLs are usually presigned, so splitting the raw + string would read the extension as ".wav?X-Amz-Signature=...". + Taking the URL path first drops the query string and the fragment. + Local paths are unaffected -- they parse as a path and nothing else. + """ + return os.path.splitext(urlparse(str(path)).path)[1].lower() + + +def file_name(path): + """ + Return the base name of a local path or a URL, without any query + string. Used for messages shown to the student, so a presigned URL + does not print its whole signature back at them. + """ + return os.path.basename(urlparse(str(path)).path) + + +def is_url(value): + """ + Return True if value is an http(s) URL rather than a local file path. + """ + return isinstance(value, str) and urlparse(value).scheme in ("http", "https") + + +def is_uploaded_file(value): + """ + Return True if value is a file object from the platform's Audio + response area: {"url", "name", "type", "size", "comment"}. + """ + return isinstance(value, dict) and isinstance(value.get("url"), str) + + +def uploaded_file_extension(file): + """ + Return the extension of an uploaded file object, taken from its URL, + then its file name, then its MIME type, whichever is recognised first. + """ + known = AUDIO_EXTENSIONS + MIDI_EXTENSIONS + for candidate in (file.get("url"), file.get("name")): + extension = file_extension(candidate or "") + if extension in known: + return extension + return MIME_TYPE_EXTENSIONS.get(file.get("type"), "") + + +@contextlib.contextmanager +def local_file(path, extension=None): + """ + Yield a local file path for path, which may be a local path or a URL. + + Basic Pitch (via librosa) and pretty_midi both need a file on disk, so + a URL has to be downloaded first. A local path is handed straight + back. A downloaded file is removed once the caller is done with it. + extension sets the temporary file's suffix, for URLs that lack one. + """ + if not is_url(path): + yield path + return + + handle, temp_path = tempfile.mkstemp(suffix=extension or file_extension(path)) + os.close(handle) + try: + with urllib.request.urlopen( + path, timeout=DOWNLOAD_TIMEOUT_SECONDS + ) as response, open(temp_path, "wb") as temp_file: + shutil.copyfileobj(response, temp_file) + yield temp_path + finally: + os.remove(temp_path) + # Helper function to check if the response is audio # --------------------------------------------------------------------- @@ -55,19 +156,19 @@ def is_audio_input(response): through the AMT pipeline before it can be compared. response can be: - - a file path string (checked by extension) + - a file path or URL string (checked by extension) - a dict that already has a "notes" key (already MIDI, skip AMT) """ # Case 1: response is already a notes dictionary, no AMT needed if isinstance(response, dict) and "notes" in response: return False - # Case 2: response is a file path string, check its extension + # Case 2: response is a file path or URL string, check its extension if isinstance(response, str): - file_extension = os.path.splitext(response)[1].lower() - if file_extension in AUDIO_EXTENSIONS: + extension = file_extension(response) + if extension in AUDIO_EXTENSIONS: return True - if file_extension in MIDI_EXTENSIONS: + if extension in MIDI_EXTENSIONS: return False # Default: if we cannot tell, assume it is not audio @@ -75,6 +176,30 @@ def is_audio_input(response): return False +# Reading MIDI files +# --------------------------------------------------------------------- +def is_midi_file_input(response): + """ + Return True if response is a path or URL to a .mid / .midi file, which + has to be read into note data rather than parsed as JSON. + """ + return isinstance(response, str) and file_extension(response) in MIDI_EXTENSIONS + + +def midi_file_to_notes(path, extension=None): + """ + Read a MIDI file (local path or URL) into the {"notes": [...]} format + expected by compare_performance_ED. Drum tracks are ignored, as they + are for transcribed audio. + """ + import pretty_midi + + with local_file(path, extension) as local_path: + midi = pretty_midi.PrettyMIDI(local_path) + + return build_compare_midi_input(convert_predicted_midi_to_notes(midi), "duration") + + # Load the model # --------------------------------------------------------------------- def load_basic_pitch_model(model_path=None): @@ -318,14 +443,19 @@ def build_compare_midi_input(notes, duration_key="duration"): # Full pipeline # --------------------------------------------------------------------- -def transcription_pipeline(audio_path, model, apply_postprocessing=True): +def transcription_pipeline(audio_path, model, apply_postprocessing=True, extension=None): """ Full production pipeline: run Basic Pitch, optionally apply post-processing, and return notes ready to hand to compare_MIDI.py. + audio_path may be a local path or a URL; a URL is downloaded to a + temporary file first. runtime_seconds covers transcription only, not + the download. + Returns (compare_midi_input, predicted_notes, runtime_seconds). """ - predicted_notes, predicted_midi, runtime_seconds = transcribe_audio(audio_path, model) + with local_file(audio_path, extension) as local_path: + predicted_notes, predicted_midi, runtime_seconds = transcribe_audio(local_path, model) if apply_postprocessing: predicted_notes = postprocess_predictions(predicted_notes) diff --git a/evaluation_function/audio_processing_test.py b/evaluation_function/audio_processing_test.py new file mode 100644 index 0000000..e7f329f --- /dev/null +++ b/evaluation_function/audio_processing_test.py @@ -0,0 +1,236 @@ +""" +audio_processing_test.py +======================== +Unit tests for reading audio input. The platform sends uploaded +recordings as URLs to a bucket, so these cover URLs as well as the +local paths used in development. + +Sections +-------- +1. Tests for file_extension, file_name and is_url +2. Tests for is_audio_input +3. Tests for local_file +4. Tests for reading MIDI files +5. Tests that every supported audio format decodes +6. Tests for uploaded file objects from the Audio response area +""" + + +import contextlib +import os +import shutil +import subprocess +import tempfile +import threading +import unittest +from functools import partial +from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer + +from .audio_processing import ( + file_extension, + file_name, + is_url, + AUDIO_EXTENSIONS, + is_audio_input, + is_midi_file_input, + is_uploaded_file, + local_file, + midi_file_to_notes, + uploaded_file_extension, +) + + +# A presigned bucket URL: the extension is followed by a query string. +SIGNED_URL = "https://bucket.example.com/uploads/rec.wav?X-Amz-Signature=abc&expires=1" + + +# 1. Tests for file_extension, file_name and is_url +# ------------------------------------------------------------------------------ +class TestPathHelpers(unittest.TestCase): + + def test_extension_of_local_path(self): + assert file_extension("/uploads/rec.wav") == ".wav" + + def test_extension_ignores_query_string(self): + assert file_extension(SIGNED_URL) == ".wav" + + def test_extension_ignores_fragment(self): + assert file_extension("https://x.example.com/rec.mp3#part2") == ".mp3" + + def test_extension_is_lowercased(self): + assert file_extension("/uploads/REC.WAV") == ".wav" + + def test_name_drops_the_signature(self): + assert file_name(SIGNED_URL) == "rec.wav" + + def test_is_url_only_for_http(self): + assert is_url("https://x.example.com/rec.wav") is True + assert is_url("http://x.example.com/rec.wav") is True + assert is_url("/uploads/rec.wav") is False + assert is_url({"notes": []}) is False + + +# 2. Tests for is_audio_input +# ------------------------------------------------------------------------------ +class TestIsAudioInput(unittest.TestCase): + + def test_signed_url_is_recognised_as_audio(self): + assert is_audio_input(SIGNED_URL) is True + + def test_local_audio_path_is_recognised(self): + assert is_audio_input("/uploads/rec.flac") is True + + def test_notes_dict_is_not_audio(self): + assert is_audio_input({"notes": []}) is False + + def test_midi_url_is_not_audio(self): + assert is_audio_input("https://x.example.com/rec.mid?sig=abc") is False + + def test_every_supported_audio_extension_is_recognised(self): + for extension in [".mp3", ".wav", ".ogg", ".aac", ".flac", ".opus", ".m4a"]: + with self.subTest(extension=extension): + assert is_audio_input(f"https://x.example.com/rec{extension}?sig=abc") is True + + +# 3. Tests for local_file +# ------------------------------------------------------------------------------ +class QuietHandler(SimpleHTTPRequestHandler): + def log_message(self, *args): + pass + + +@contextlib.contextmanager +def serve_directory(directory): + """Serve directory over HTTP on a free local port; yield its base URL.""" + handler = partial(QuietHandler, directory=directory) + server = ThreadingHTTPServer(("127.0.0.1", 0), handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + try: + yield f"http://127.0.0.1:{server.server_port}" + finally: + server.shutdown() + + +class TestLocalFile(unittest.TestCase): + + CONTENT = b"stand-in for audio bytes" + + def test_local_path_is_handed_back_unchanged(self): + with local_file("/uploads/rec.wav") as path: + assert path == "/uploads/rec.wav" + + def test_url_is_downloaded_and_removed_afterwards(self): + with tempfile.TemporaryDirectory() as served_directory: + with open(os.path.join(served_directory, "rec.wav"), "wb") as f: + f.write(self.CONTENT) + + handler = partial(QuietHandler, directory=served_directory) + server = ThreadingHTTPServer(("127.0.0.1", 0), handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + + try: + url = f"http://127.0.0.1:{server.server_port}/rec.wav?X-Amz-Signature=abc" + with local_file(url) as path: + # Basic Pitch picks its decoder by extension, so the + # temporary file has to keep the .wav suffix. + assert path.endswith(".wav") + with open(path, "rb") as f: + assert f.read() == self.CONTENT + downloaded_path = path + + assert not os.path.exists(downloaded_path) + finally: + server.shutdown() + + +# 4. Tests for reading MIDI files +# ------------------------------------------------------------------------------ +def write_midi(path, notes): + """Write (pitch, start, end) tuples to a MIDI file, plus one drum hit.""" + import pretty_midi + + midi = pretty_midi.PrettyMIDI() + piano = pretty_midi.Instrument(program=0) + for pitch, start, end in notes: + piano.notes.append(pretty_midi.Note(velocity=80, pitch=pitch, start=start, end=end)) + drums = pretty_midi.Instrument(program=0, is_drum=True) + drums.notes.append(pretty_midi.Note(velocity=80, pitch=36, start=0.0, end=0.1)) + midi.instruments.extend([piano, drums]) + midi.write(path) + + +class TestMidiFiles(unittest.TestCase): + + NOTES = [(64, 0.5, 1.0), (60, 0.0, 0.5)] + + def test_midi_paths_and_urls_are_recognised(self): + assert is_midi_file_input("/uploads/rec.mid") is True + assert is_midi_file_input("https://x.example.com/rec.MIDI?sig=abc") is True + assert is_midi_file_input("https://x.example.com/rec.wav") is False + assert is_midi_file_input('{"notes": []}') is False + assert is_midi_file_input({"notes": []}) is False + + def test_reads_notes_from_a_url_ignoring_drums(self): + with tempfile.TemporaryDirectory() as directory: + write_midi(os.path.join(directory, "rec.mid"), self.NOTES) + with serve_directory(directory) as base_url: + result = midi_file_to_notes(f"{base_url}/rec.mid?X-Amz-Signature=abc") + + assert [note["pitch"] for note in result["notes"]] == [60, 64] + assert result["notes"][1]["start"] == 0.5 + assert abs(result["notes"][1]["duration"] - 0.5) < 1e-6 + + +# 5. Tests that every supported audio format decodes +# ------------------------------------------------------------------------------ +@unittest.skipUnless(shutil.which("ffmpeg"), "ffmpeg is needed to create and decode the samples") +class TestAudioFormatsDecode(unittest.TestCase): + """ + Basic Pitch loads audio with librosa. Encode a short tone in each + supported format and check librosa can read it back, so a missing + decoder (e.g. no ffmpeg for m4a/aac) shows up here rather than on a + student's submission. + """ + + def test_each_format_decodes(self): + import librosa + + with tempfile.TemporaryDirectory() as directory: + for extension in AUDIO_EXTENSIONS: + with self.subTest(extension=extension): + path = os.path.join(directory, "tone" + extension) + subprocess.run( + ["ffmpeg", "-loglevel", "error", "-f", "lavfi", + "-i", "sine=frequency=440:duration=0.5", path], + check=True, + ) + samples, _ = librosa.load(path, sr=22050, mono=True) + assert len(samples) > 0 + + +# 6. Tests for uploaded file objects from the Audio response area +# ------------------------------------------------------------------------------ +class TestUploadedFileExtension(unittest.TestCase): + + def upload(self, url, name="", type=""): + return {"url": url, "name": name, "type": type, "size": 1, "comment": ""} + + def test_is_uploaded_file(self): + assert is_uploaded_file(self.upload("https://x.example.com/a.wav")) is True + assert is_uploaded_file({"notes": []}) is False + assert is_uploaded_file("https://x.example.com/a.wav") is False + + def test_extension_from_the_url(self): + assert uploaded_file_extension(self.upload("https://x.example.com/a.mp3?sig=1")) == ".mp3" + + def test_extension_from_the_name_when_the_url_has_none(self): + upload = self.upload("https://x.example.com/abc123", name="Take 2.FLAC") + assert uploaded_file_extension(upload) == ".flac" + + def test_extension_from_the_mime_type_as_a_last_resort(self): + upload = self.upload("https://x.example.com/abc123", name="recording", type="audio/mp4") + assert uploaded_file_extension(upload) == ".m4a" + + def test_unknown_file_has_no_extension(self): + upload = self.upload("https://x.example.com/a.pdf", name="a.pdf", type="application/pdf") + assert uploaded_file_extension(upload) == "" diff --git a/evaluation_function/evaluation.py b/evaluation_function/evaluation.py index 8bc18b1..305f498 100755 --- a/evaluation_function/evaluation.py +++ b/evaluation_function/evaluation.py @@ -20,9 +20,16 @@ DEFAULT_CHORD_ONSET_WINDOW ) from .audio_processing import ( + AUDIO_EXTENSIONS, + file_extension, + file_name, is_audio_input, + is_midi_file_input, + is_uploaded_file, load_basic_pitch_model, + midi_file_to_notes, transcription_pipeline, + uploaded_file_extension, ) # The Basic Pitch model is loaded lazily, on the first audio request, rather @@ -50,24 +57,58 @@ def parse_json_input(data: Any) -> Any: return data +class UnsupportedFileError(ValueError): + """An uploaded file is not in a format this function can read.""" + + +def read_file(source: str, extension: str) -> Dict[str, Any]: + """Read an audio or MIDI file (path or URL) into note data.""" + if extension in AUDIO_EXTENSIONS: + compare_midi_input, predicted_notes, runtime_seconds = transcription_pipeline( + source, get_basic_pitch_model(), extension=extension + ) + return compare_midi_input + return midi_file_to_notes(source, extension) + + def prepare_input(raw_input: Any) -> Any: """ Turn one raw input (either response or answer) into the format that compare_performance_ED expects. - There are two possible cases: - - raw_input is an audio file path + The possible cases are: + - raw_input is an audio file path or URL -> run it through the AMT (audio-to-MIDI) transcription pipeline - - raw_input is already MIDI (a dict, or a JSON string of a dict) - -> just parse it as JSON, no transcription needed + - raw_input is a .mid / .midi file path or URL + -> read the notes from the file, no transcription needed + - raw_input is a list of uploaded files from the platform's Audio + response area, [{"url", "name", "type", "size", "comment"}, ...] + -> read the first file as above; an empty list has no notes + - raw_input is already note data (a dict, or a JSON string of a dict) + -> just parse it as JSON """ - if is_audio_input(raw_input): - compare_midi_input, predicted_notes, runtime_seconds = transcription_pipeline( - raw_input, get_basic_pitch_model() - ) - return compare_midi_input + if is_audio_input(raw_input) or is_midi_file_input(raw_input): + return read_file(raw_input, file_extension(raw_input)) + + data = parse_json_input(raw_input) + + if isinstance(data, list): + if not data: + return {"notes": []} + # The response area allows several files, but a submission is one + # performance, so only the first file is marked. + data = data[0] + + if is_uploaded_file(data): + extension = uploaded_file_extension(data) + if not extension: + raise UnsupportedFileError( + f"{data.get('name') or file_name(data['url'])} is not a supported " + "audio or MIDI file." + ) + return read_file(data["url"], extension) - return parse_json_input(raw_input) + return data def evaluation_function( @@ -102,8 +143,11 @@ def evaluation_function( # The Lambda Feedback app sends response/answer as JSON strings, # convert them into Python object first - response = prepare_input(response) - answer = prepare_input(answer) + try: + response = prepare_input(response) + answer = prepare_input(answer) + except UnsupportedFileError as error: + return {"is_correct": False, "feedback": str(error)} result = compare_performance_ED( response, diff --git a/evaluation_function/evaluation_test.py b/evaluation_function/evaluation_test.py index 17bc3c1..2b5c01d 100755 --- a/evaluation_function/evaluation_test.py +++ b/evaluation_function/evaluation_test.py @@ -832,3 +832,70 @@ def test_two_notes_against_four_note_reference(self): def test_single_note_matching_single_note_reference_is_correct(self): one_note = short_melody(1) assert compare_performance_ED(one_note, one_note).is_correct is True + + +class TestMidiFileInput(unittest.TestCase): + """A .mid path or URL is read into note data before comparison.""" + + def test_midi_file_matches_the_same_notes_as_data(self): + import os + import tempfile + + from .audio_processing_test import write_midi + + notes = [(60, 0.0, 0.5), (62, 0.5, 1.0), (64, 1.0, 1.5), (65, 1.5, 2.0)] + reference = make_midi( + [n[0] for n in notes], [n[1] for n in notes], [n[2] - n[1] for n in notes] + ) + + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "performance.mid") + write_midi(path, notes) + result = evaluation_function(path, reference, {}) + + assert result["is_correct"] is True + + +class TestUploadedFileInput(unittest.TestCase): + """ + The platform's Audio response area sends both response and answer as a + list of uploaded files: [{"url", "name", "type", "size", "comment"}]. + """ + + NOTES = [(60, 0.0, 0.5), (62, 0.5, 1.0), (64, 1.0, 1.5), (65, 1.5, 2.0)] + + def setUp(self): + import os + import tempfile + + from .audio_processing_test import write_midi + + self.directory = tempfile.TemporaryDirectory() + self.midi_path = os.path.join(self.directory.name, "performance.mid") + write_midi(self.midi_path, self.NOTES) + + def tearDown(self): + self.directory.cleanup() + + def upload(self, url, name="performance.mid", type="audio/midi"): + return [{"url": url, "name": name, "type": type, "size": 162, "comment": ""}] + + def test_uploaded_midi_matches_the_same_uploaded_midi(self): + result = evaluation_function(self.upload(self.midi_path), self.upload(self.midi_path), {}) + assert result["is_correct"] is True + + def test_empty_answer_list_reports_a_misconfigured_question(self): + result = evaluation_function(self.upload(self.midi_path), [], {}) + assert result["is_correct"] is False + assert result["feedback"] == NO_REFERENCE_NOTES_MESSAGE + + def test_empty_response_list_reports_no_submission(self): + result = evaluation_function([], self.upload(self.midi_path), {}) + assert result["is_correct"] is False + assert result["feedback"] == NO_RESPONSE_NOTES_MESSAGE + + def test_unsupported_file_is_reported_not_raised(self): + upload = self.upload("https://x.example.com/notes.pdf", "notes.pdf", "application/pdf") + result = evaluation_function(upload, self.upload(self.midi_path), {}) + assert result["is_correct"] is False + assert "notes.pdf" in result["feedback"] diff --git a/evaluation_function/preview.py b/evaluation_function/preview.py index 35c2bcb..86ca2d4 100755 --- a/evaluation_function/preview.py +++ b/evaluation_function/preview.py @@ -13,12 +13,20 @@ """ import json -import os from typing import Any from lf_toolkit.preview import Result, Params, Preview -from .audio_processing import AUDIO_EXTENSIONS +from .audio_processing import ( + AUDIO_EXTENSIONS, + MIDI_EXTENSIONS, + file_extension, + file_name, + is_midi_file_input, + is_uploaded_file, + midi_file_to_notes, + uploaded_file_extension, +) from .compare_MIDI import PITCH_CLASS_NAMES # Fixed messages, named so that tests can assert which case was hit without @@ -68,14 +76,36 @@ def preview_function(response: Any, params: Params) -> Result: # An audio recording is reported as such. Transcribing it here would # take seconds, which is far too slow while the student is working. if isinstance(response, str): - extension = os.path.splitext(response)[1].lower() - if extension in AUDIO_EXTENSIONS: - name = os.path.basename(response) + if file_extension(response) in AUDIO_EXTENSIONS: + name = file_name(response) return Result(preview=Preview( feedback=f"Audio recording {name}, transcribed on submission." )) - response = json.loads(response) + # A MIDI file is small and quick to read, so it is summarised + # like note data rather than just named. + if is_midi_file_input(response): + response = midi_file_to_notes(response) + else: + response = json.loads(response) + + # The platform's Audio response area sends a list of uploaded files; + # only the first one is marked. + if isinstance(response, list): + if not response: + return Result(preview=Preview(feedback=NO_NOTES_MESSAGE)) + response = response[0] + + if is_uploaded_file(response): + extension = uploaded_file_extension(response) + name = response.get("name") or file_name(response["url"]) + if extension in AUDIO_EXTENSIONS: + return Result(preview=Preview( + feedback=f"Audio recording {name}, transcribed on submission." + )) + if extension not in MIDI_EXTENSIONS: + return Result(preview=Preview(feedback=UNREADABLE_MESSAGE)) + response = midi_file_to_notes(response["url"], extension) return Result(preview=Preview(feedback=summarise_notes(response["notes"]))) diff --git a/evaluation_function/preview_test.py b/evaluation_function/preview_test.py index e41307a..dba5de6 100755 --- a/evaluation_function/preview_test.py +++ b/evaluation_function/preview_test.py @@ -15,8 +15,12 @@ """ import json +import os +import tempfile import unittest +from .audio_processing_test import write_midi + from .evaluation_test import make_midi from .preview import ( NO_NOTES_MESSAGE, @@ -72,6 +76,14 @@ def test_audio_path_is_described_without_transcribing(self): assert "audio" in feedback.lower() assert "practice.wav" in feedback + def test_signed_url_is_described_without_the_signature(self): + feedback = feedback_for( + "https://bucket.example.com/uploads/practice.wav?X-Amz-Signature=abc" + ) + assert "audio" in feedback.lower() + assert "practice.wav" in feedback + assert "X-Amz-Signature" not in feedback + def test_unreadable_submission_is_reported(self): assert feedback_for("this is not MIDI at all") == UNREADABLE_MESSAGE @@ -92,3 +104,35 @@ def test_octave_above_middle_c(self): def test_accidental(self): assert note_name(61) == "C#4" + + +class TestMidiFileSubmissions(unittest.TestCase): + + def test_midi_file_is_summarised_like_note_data(self): + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "practice.mid") + write_midi(path, [(60, 0.0, 0.5), (64, 0.5, 1.0)]) + assert "2 notes" in feedback_for(path) + + +class TestUploadedFileSubmissions(unittest.TestCase): + """The Audio response area sends a list of uploaded file objects.""" + + def upload(self, url, name, type): + return [{"url": url, "name": name, "type": type, "size": 1, "comment": ""}] + + def test_empty_upload_list_has_no_notes(self): + assert feedback_for([]) == NO_NOTES_MESSAGE + + def test_uploaded_audio_is_named_not_transcribed(self): + feedback = feedback_for(self.upload( + "https://bucket.example.com/abc.wav?X-Amz-Signature=abc", "practice.wav", "audio/wav" + )) + assert "audio" in feedback.lower() + assert "practice.wav" in feedback + + def test_uploaded_midi_is_summarised(self): + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "practice.mid") + write_midi(path, [(60, 0.0, 0.5), (64, 0.5, 1.0)]) + assert "2 notes" in feedback_for(self.upload(path, "practice.mid", "audio/midi"))