diff --git a/CMakeLists.txt b/CMakeLists.txt index 8acb10b..b292d3b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -255,6 +255,11 @@ loopercat_core_test(connect_gate_tests) # What the app remembers about the pedals it has met (issue #98): pure, so # it is a core suite though it lives in app/. loopercat_core_test(pedal_book_tests) +# What a file name on the card may be (FatName.hpp): FAT's rules and Windows's. +loopercat_core_test(fat_name_tests) +# The name a pushed file lands under (issue #139): pure, so it is a core +# suite though it lives in app/. +loopercat_core_test(upload_mark_tests) # The empty window's words about the bus, by the profile table's read gate. loopercat_core_test(pedal_presence_tests) loopercat_core_test(params_tests) @@ -714,6 +719,25 @@ target_link_libraries(wav_import_tests PRIVATE juce::juce_recommended_warning_flags) add_test(NAME wav_import_tests COMMAND wav_import_tests) +# ---------------------------------------------------------------------------- +# The upload preferences (issues #53, #139) in the app's own PropertiesFile: +# defaults for an untouched file, round trips through a fresh open, a mark +# cleared to nothing staying nothing. +# ---------------------------------------------------------------------------- +juce_add_console_app(import_prefs_tests PRODUCT_NAME "import_prefs_tests") +target_sources(import_prefs_tests PRIVATE tests/import_prefs_tests.cpp) +target_compile_definitions(import_prefs_tests PRIVATE + JUCE_WEB_BROWSER=0 + JUCE_USE_CURL=0 + LOOPERCAT_GOLDEN_JSON="${CMAKE_CURRENT_SOURCE_DIR}/fixtures/golden.json") +target_link_libraries(import_prefs_tests PRIVATE + loopercat::core + nlohmann_json::nlohmann_json + juce::juce_gui_basics + juce::juce_recommended_config_flags + juce::juce_recommended_warning_flags) +add_test(NAME import_prefs_tests COMMAND import_prefs_tests) + # ---------------------------------------------------------------------------- # The history store (issue #72): theory tests against real SQLite files — the # storage properties read back, atomicity of a take and its row, dedup, diff --git a/app/ImportPrefs.h b/app/ImportPrefs.h new file mode 100644 index 0000000..b6a9217 --- /dev/null +++ b/app/ImportPrefs.h @@ -0,0 +1,86 @@ +// Copyright (c) 2026 Darwin's Cat. Part of Looper Cat — see LICENSE. +// SPDX-License-Identifier: AGPL-3.0-or-later + +#pragma once + +#include "UploadMark.h" + +#include + +//============================================================================== +// loopercat::ImportPrefs — what happens to an upload on its way to the card, +// and where the app keeps the answer: three keys in the per-user +// PropertiesFile (AppSettings), beside the column and history preferences. +// +// One reader and one writer, so the Settings dialog that edits them, the push +// job that acts on them and a test that round-trips them all see the same +// keys with the same defaults. The push job reads them when the player acts +// — as the job is queued — and carries them with it: a settings change +// after that must not rewrite a job already promised. +//============================================================================== +namespace loopercat +{ + +struct ImportPrefs { + // Normalize-on-upload (issue #53): OFF by default so every existing + // workflow keeps the byte-exact pass-through; -18 LUFS is ReplayGain + // 2.0's reference — the modern spelling of the "89 dB" the request + // arrived in. + bool normalizeOnUpload; + double targetLufs; + // The mark a converted upload lands with (issue #139): "-pedal" out of + // the box, which is the spelling existing cards already carry; empty + // means converted uploads land under their own name. + juce::String convertedMark; +}; + +namespace importprefs +{ + // The one rewrite a mark gets, typed or stored: a mark of only + // whitespace is the empty mark — "Empty: no mark", as the hint says. + // Any other mark stays as typed, spaces before, inside and after + // included: a rewritten mark is a different mark (UploadMark.h). + inline juce::String markAsTyped(const juce::String& text) + { + return text.trim().isEmpty() ? juce::String() : text; + } + + inline constexpr auto kNormalizeOnUploadKey = "normalizeOnUpload"; + inline constexpr auto kTargetLufsKey = "normalizeTargetLufs"; + inline constexpr auto kConvertedMarkKey = "convertedUploadMark"; + inline constexpr double kDefaultTargetLufs = -18.0; + + // What a player who never opened Settings gets. + inline ImportPrefs defaults() + { + return { .normalizeOnUpload = false, + .targetLufs = kDefaultTargetLufs, + .convertedMark = juce::String(uploadmark::kDefaultSuffix.data(), + uploadmark::kDefaultSuffix.size()) }; + } + + // A key that is absent reads as its default; a key that is present reads + // as stored, so a mark cleared to "" stays cleared — PropertySet::getValue + // falls back only when the key is missing, not when its value is empty. + // The mark gets the field's one rule (markAsTyped), so "only spaces is + // the empty mark" holds whichever hand wrote the file — and nothing else + // about it is rewritten. + inline ImportPrefs read(juce::PropertiesFile& file) + { + const ImportPrefs shipped = defaults(); + return { .normalizeOnUpload = + file.getBoolValue(kNormalizeOnUploadKey, shipped.normalizeOnUpload), + .targetLufs = file.getDoubleValue(kTargetLufsKey, shipped.targetLufs), + .convertedMark = markAsTyped(file.getValue(kConvertedMarkKey, shipped.convertedMark)) }; + } + + inline void write(juce::PropertiesFile& file, const ImportPrefs& prefs) + { + file.setValue(kNormalizeOnUploadKey, prefs.normalizeOnUpload); + file.setValue(kTargetLufsKey, prefs.targetLufs); + file.setValue(kConvertedMarkKey, prefs.convertedMark); + file.saveIfNeeded(); + } +} // namespace importprefs + +} // namespace loopercat diff --git a/app/MainComponent.cpp b/app/MainComponent.cpp index 7a7b5e3..2204172 100644 --- a/app/MainComponent.cpp +++ b/app/MainComponent.cpp @@ -14,6 +14,8 @@ #include "OperationsLog.h" #include "PedalPortName.h" #include "Strings.h" +#include "ImportPrefs.h" +#include "UploadMark.h" #include "WavImport.h" #include "CardPermissions.h" @@ -48,13 +50,10 @@ namespace // until a measure or a check fills it, and an empty column is noise. constexpr auto kLoudnessColumnKey = "columnLoudness"; - // Normalize-on-upload (issue #53): OFF by default so every existing - // workflow keeps the byte-exact pass-through; -18 LUFS is ReplayGain - // 2.0's reference — the modern spelling of the "89 dB" the request - // arrived in. - constexpr auto kNormalizeOnUploadKey = "normalizeOnUpload"; - constexpr auto kNormalizeTargetLufsKey = "normalizeTargetLufs"; - constexpr double kDefaultTargetLufs = -18.0; + // The upload preferences live in ImportPrefs.h (issues #53, #139); the + // Normalize commands read the target under its name here. + constexpr auto kNormalizeTargetLufsKey = importprefs::kTargetLufsKey; + constexpr double kDefaultTargetLufs = importprefs::kDefaultTargetLufs; // The history's storage limit (issue #74): the bytes of takes the app // keeps before it starts offering the oldest for release. 5 GB out of @@ -2076,50 +2075,62 @@ void MainComponent::pushWav(int slot, const juce::String& sourcePath, bool slotO { const auto enqueuePush = [this, slot, sourcePath](bool force) { releasePlayerIfHolding(slot, slot); // a replace rewrites the WAV under preview (issue #26) - // The normalize preference is read HERE, when the player acts — a + // The upload preferences are read HERE, when the player acts — a // settings change mid-queue must not rewrite jobs already promised. - std::optional normalizeTarget; - if (auto* file = settings.file(); - file != nullptr && file->getBoolValue(kNormalizeOnUploadKey, false)) - normalizeTarget = file->getDoubleValue(kNormalizeTargetLufsKey, kDefaultTargetLufs); + auto* file = settings.file(); + const ImportPrefs prefs = file != nullptr ? importprefs::read(*file) : importprefs::defaults(); + const std::optional normalizeTarget = + prefs.normalizeOnUpload ? std::optional(prefs.targetLufs) : std::nullopt; auto note = std::make_shared(); const auto options = makeWriteOptions(); worker.enqueue(recorded("push", options, { "Push " + juce::File(sourcePath).getFileName() + " to slot " + juce::String(slot), slot, [source = sourcePath, slot, force, normalizeTarget, note, - options, + mark = prefs.convertedMark.toStdString(), options, importTmp = settings.dataDir().getChildFile("import-tmp"), logDir = settings.dataDir()]( const volume::fs::path& volumePath) { // DAW exports arrive as anything — convert off the // message thread, on this worker, before the push - // (issue #20). The temp conversion dies with the job. + // (issue #20). The conversion's directory dies with + // `prepared`, on success and on a throw alike. + const juce::String sourceName = juce::File(source).getFileName(); wavimport::Prepared prepared; const juce::Result ok = wavimport::prepare(juce::File(source), importTmp, prepared, { .normalizeTargetLufs = normalizeTarget }); if (ok.failed()) throw Error(ok.getErrorMessage().toStdString()); - commands::PushResult pushed; - try { - pushed = commands::push(volumePath, - prepared.file.getFullPathName().toStdString(), - slot, - { .force = force, .write = options }); - } catch (...) { - if (prepared.converted) - prepared.file.deleteFile(); - throw; - } - if (prepared.converted) - prepared.file.deleteFile(); - if (prepared.normalize.has_value() && normalizeTarget.has_value()) { - *note = describeNormalize(*prepared.normalize, *normalizeTarget); + // The name on the card is decided here, not inherited + // from the conversion's temp file (issue #139): a + // rebuilt upload carries the mark, one only repacked + // into the pedal's container lands under its own + // stem, an untouched one keeps its name. + const uploadmark::Audio audio = !prepared.converted + ? uploadmark::Audio::untouched + : prepared.rebuilt ? uploadmark::Audio::rebuilt + : uploadmark::Audio::repackaged; + const std::string landed = + uploadmark::landedName(sourceName.toStdString(), mark, audio); + const commands::PushResult pushed = commands::push( + volumePath, prepared.file.getFullPathName().toStdString(), slot, + { .landedName = landed, .force = force, .write = options }); + // What changed on the way to the card: the rebuild's + // facts, then the normalization's — each only when it + // has something to say. + if (prepared.sourceFormat.has_value()) + *note = wavimport::describeConversion(*prepared.sourceFormat); + if (prepared.normalize.has_value() && normalizeTarget.has_value()) + *note << (note->isEmpty() ? "" : "; ") + << describeNormalize(*prepared.normalize, *normalizeTarget); + if (note->isNotEmpty()) { + const juce::String as = juce::String(landed) == sourceName + ? juce::String() + : " as " + juce::String(landed); oplog::append(logDir, - "push " + juce::File(source).getFileName() - + " to slot " + juce::String(slot) + ": " - + *note); + "push " + sourceName + " to slot " + juce::String(slot) + + as + ": " + *note); } // The slot's length was a note value and this take // replaced it with a bar count (issue #92): said out @@ -2616,17 +2627,10 @@ std::unique_ptr MainComponent::makeSettingsDialog() } applyColumnPreferences(); }, - SettingsDialog::ImportPrefs { - settings.file() != nullptr && settings.file()->getBoolValue(kNormalizeOnUploadKey, false), - settings.file() != nullptr - ? settings.file()->getDoubleValue(kNormalizeTargetLufsKey, kDefaultTargetLufs) - : kDefaultTargetLufs }, + settings.file() != nullptr ? importprefs::read(*settings.file()) : importprefs::defaults(), [this](SettingsDialog::ImportPrefs prefs) { - if (auto* file = settings.file()) { - file->setValue(kNormalizeOnUploadKey, prefs.normalizeOnUpload); - file->setValue(kNormalizeTargetLufsKey, prefs.targetLufs); - file->saveIfNeeded(); - } + if (auto* file = settings.file()) + importprefs::write(*file, prefs); }); // The storage panel (issue #74) is fed by the worker, where the store diff --git a/app/Mp3AudioFormat.cpp b/app/Mp3AudioFormat.cpp index 88efb6c..4cd5f98 100644 --- a/app/Mp3AudioFormat.cpp +++ b/app/Mp3AudioFormat.cpp @@ -26,7 +26,7 @@ namespace { public: explicit Mp3Reader(juce::InputStream* stream) - : juce::AudioFormatReader(stream, "MP3") + : juce::AudioFormatReader(stream, Mp3AudioFormat::kReaderName) { input->readIntoMemoryBlock(data); if (mp3dec_ex_open_buf(&dec, static_cast(data.getData()), diff --git a/app/Mp3AudioFormat.h b/app/Mp3AudioFormat.h index 6d78d82..26a6d69 100644 --- a/app/Mp3AudioFormat.h +++ b/app/Mp3AudioFormat.h @@ -27,6 +27,10 @@ namespace loopercat class Mp3AudioFormat final : public juce::AudioFormat { public: + // What the reader calls itself (AudioFormatReader::getFormatName): the + // import's report tells a codec's reader from a sample reader by this. + static constexpr const char* kReaderName = "MP3"; + Mp3AudioFormat(); juce::Array getPossibleSampleRates() override; diff --git a/app/SettingsDialog.h b/app/SettingsDialog.h index f18f819..9c3b67b 100644 --- a/app/SettingsDialog.h +++ b/app/SettingsDialog.h @@ -5,7 +5,11 @@ #include "AppSettings.h" #include "HistoryStoragePanel.h" +#include "ImportPrefs.h" #include "TabStrip.h" +#include "UploadMark.h" + +#include #include @@ -32,6 +36,12 @@ // this one. The target is stored in LUFS; the label translates it into // ReplayGain vocabulary (reference 89 dB = -18 LUFS, the linear offset is // +107) because "normalize to 89 dB" is how the request arrived. +// +// The mark on converted uploads (issue #139) ships ON as "-pedal": that is +// the spelling existing cards already carry, from the days the conversion's +// temp-file name travelled to the card by accident. The field refuses a +// character a FAT name cannot hold and snaps back to the stored mark, with +// the refusal said under it — never a silently rewritten mark. //============================================================================== namespace loopercat { @@ -45,10 +55,7 @@ class SettingsDialog final : public juce::Component bool loudness; // the LUFS column (issue #61) — off until asked for }; - struct ImportPrefs { - bool normalizeOnUpload; - double targetLufs; - }; + using ImportPrefs = loopercat::ImportPrefs; // read and written by importprefs:: // The target field refuses values outside this window: hotter than -8 // leaves no headroom against a live band's transients, quieter than -30 @@ -123,6 +130,31 @@ class SettingsDialog final : public juce::Component importHint_.setColour(juce::Label::textColourId, juce::Colour(0xff6f6f78)); addChildComponent(importHint_); + markCaption_.setText("Mark uploads converted for the pedal with", juce::dontSendNotification); + markCaption_.setFont(juce::FontOptions(12.0f)); + markCaption_.setColour(juce::Label::textColourId, juce::Colour(0xff8a8a92)); + addChildComponent(markCaption_); + + // No input restriction: a forbidden character is refused with its name, + // not dropped on the way in — the player sees what was wrong with it. + mark_.setText(importPrefs_.convertedMark, juce::dontSendNotification); + mark_.onReturnKey = [this] { parseMark(); }; + mark_.onFocusLost = [this] { parseMark(); }; + addChildComponent(mark_); + + markRefusal_.setFont(juce::FontOptions(11.0f)); + markRefusal_.setColour(juce::Label::textColourId, juce::Colour(0xffff8a3d)); // brand orange + addChildComponent(markRefusal_); + + markHint_.setText("Added to the file name on the card when the audio had to be " + "rebuilt for the pedal: a different sample rate, bit depth or " + "channel count, or a normalization that rewrote it. A file the " + "pedal takes as-is keeps its name. Empty: no mark.", + juce::dontSendNotification); + markHint_.setFont(juce::FontOptions(11.0f)); + markHint_.setColour(juce::Label::textColourId, juce::Colour(0xff6f6f78)); + addChildComponent(markHint_); + addChildComponent(storage_); addAndMakeVisible(tabs_); @@ -180,6 +212,13 @@ class SettingsDialog final : public juce::Component targetEquiv_.setBounds(targetRow); importArea.removeFromTop(12); importHint_.setBounds(importArea.removeFromTop(40)); + importArea.removeFromTop(14); + markCaption_.setBounds(importArea.removeFromTop(20)); + importArea.removeFromTop(4); + mark_.setBounds(importArea.removeFromTop(24).removeFromLeft(160)); + importArea.removeFromTop(4); + markRefusal_.setBounds(importArea.removeFromTop(18)); + markHint_.setBounds(importArea.removeFromTop(54)); } private: @@ -191,7 +230,8 @@ class SettingsDialog final : public juce::Component countIn_.setVisible(index == 1); loudness_.setVisible(index == 1); for (auto* c : std::initializer_list { - &normalize_, &targetCaption_, &target_, &targetEquiv_, &importHint_ }) + &normalize_, &targetCaption_, &target_, &targetEquiv_, &importHint_, + &markCaption_, &mark_, &markRefusal_, &markHint_ }) c->setVisible(index == 2); storage_.setVisible(index == kHistoryTab); } @@ -234,6 +274,29 @@ class SettingsDialog final : public juce::Component commitImport(); } + void parseMark() + { + // A mark of only spaces is the empty mark the hint promises, not a + // name with a space before its extension; every other mark is kept + // as typed, its spaces included — a FAT name may hold them, and a + // rewritten mark would be a different mark. + const juce::String typed = importprefs::markAsTyped(mark_.getText()); + try { + uploadmark::assertSuffix(typed.toStdString()); + } catch (const Error& refused) { + // Not a mark — snap back to the stored one, and say why. + mark_.setText(importPrefs_.convertedMark, juce::dontSendNotification); + markRefusal_.setText(juce::String::fromUTF8(refused.what()), juce::dontSendNotification); + return; + } + markRefusal_.setText({}, juce::dontSendNotification); + mark_.setText(typed, juce::dontSendNotification); // what was accepted, as it will be stored + if (typed == importPrefs_.convertedMark) + return; + importPrefs_.convertedMark = typed; + commitImport(); + } + static constexpr int kHistoryTab = 3; TabStrip tabs_ { { "Audio", "Columns", "Import", "History" } }; felitronics::appkit::AudioSettingsPanel audio_; @@ -251,6 +314,10 @@ class SettingsDialog final : public juce::Component juce::TextEditor target_; juce::Label targetEquiv_; juce::Label importHint_; + juce::Label markCaption_; + juce::TextEditor mark_; + juce::Label markRefusal_; + juce::Label markHint_; HistoryStoragePanel storage_; diff --git a/app/UploadMark.h b/app/UploadMark.h new file mode 100644 index 0000000..c138a93 --- /dev/null +++ b/app/UploadMark.h @@ -0,0 +1,105 @@ +// Copyright (c) 2026 Darwin's Cat. Part of Looper Cat — see LICENSE. +// SPDX-License-Identifier: AGPL-3.0-or-later + +#pragma once + +#include +#include + +#include +#include +#include + +//============================================================================== +// loopercat::uploadmark — the name a pushed file lands under on the card +// (issue #139). Pure: no files, no JUCE, so the rule is tested as a rule. +// +// An upload the pedal takes as-is keeps its own name. One whose samples had +// to be rebuilt — resampled, spread to two channels, re-quantised, +// gain-adjusted — lands as .wav: the bytes in that slot +// are not the bytes of the player's file, and a take pulled back a month +// later should say so at a glance. One that was only repacked into the +// pedal's container — a float export wearing a header the pedal's gate does +// not read, every sample intact — lands as .wav: the extension +// is changed on purpose, so an imported song.mp3 becomes song.wav rather than +// keeping an extension that lies about what is inside, but there is nothing +// to mark. +// +// The suffix is the player's (Settings -> Import); empty means no mark. Before +// this rule existed the suffix was the name of the conversion's temp file, +// travelling to the card by accident — the default keeps that spelling so an +// existing card keeps the names it already has. +//============================================================================== +namespace loopercat::uploadmark +{ + +inline constexpr std::string_view kDefaultSuffix = "-pedal"; +inline constexpr std::string_view kExtension = ".wav"; + +// A mark longer than this leaves no room for a name in front of it: the +// card's limit (fatname::kMaxLandedUnits, what Windows's MAX_PATH leaves +// under the card folder), less the extension, less one character of stem. +inline constexpr std::size_t kMaxSuffixUnits = fatname::kMaxLandedUnits - kExtension.size() - 1; +static_assert(kMaxSuffixUnits == 233); + +// What happened to the audio on its way to the card. +enum class Audio { + untouched, // the source file itself goes to the card: its name stays + repackaged, // the same samples in the pedal's own container: .wav, no mark + rebuilt // the samples changed: .wav +}; + +// Refuses, with the character named, rather than stripping it: a mark the +// player typed and a mark the app silently rewrote are two different marks. +// The cap is a named one so the Settings field can refuse a mark no name +// could carry. +inline void assertSuffix(std::string_view suffix) +{ + if (const auto c = fatname::forbiddenCharacter(suffix)) + throw Error("the mark cannot contain " + fatname::describeCharacter(*c) + + " — a file name on the card cannot hold it"); + const std::size_t units = fatname::utf16Units(suffix); + if (units > kMaxSuffixUnits) + throw Error("the mark is " + std::to_string(units) + " characters long; it can be at most " + + std::to_string(kMaxSuffixUnits) + " — a file name on the card holds " + + std::to_string(fatname::kMaxLandedUnits) + ", and the name needs room in front"); +} + +// The stem is everything before the last dot; a name with no dot, or only a +// leading one, is all stem — "v1.2 mix.aif" keeps its "1.2". Done by hand +// rather than through std::filesystem so a UTF-8 name survives byte-exact on +// every platform. +inline std::string stem(std::string_view fileName) +{ + const auto dot = fileName.rfind('.'); + if (dot == std::string_view::npos || dot == 0) + return std::string(fileName); + return std::string(fileName.substr(0, dot)); +} + +// The file name on the card for `sourceFileName`, by what happened to its +// audio. The suffix is judged only where it is used — a mark that was +// hand-edited into the settings file must not stop an untouched upload. +// A name longer than a FAT name can be is refused, never trimmed: the core +// would refuse it before a byte moved, and the player should hear why from +// the app, with the name it would have been. +inline std::string landedName(std::string_view sourceFileName, std::string_view suffix, + Audio audio) +{ + if (audio == Audio::untouched) + return std::string(sourceFileName); + if (audio == Audio::rebuilt) + assertSuffix(suffix); + const std::string name = stem(sourceFileName) + + (audio == Audio::rebuilt ? std::string(suffix) : std::string()) + + std::string(kExtension); + const std::size_t units = fatname::utf16Units(name); + if (units > fatname::kMaxLandedUnits) + throw Error("\"" + name + "\" is " + std::to_string(units) + + " characters long; a file name on the card holds at most " + + std::to_string(fatname::kMaxLandedUnits) + " — shorten the file name" + + (audio == Audio::rebuilt ? " or the mark" : "")); + return name; +} + +} // namespace loopercat::uploadmark diff --git a/app/WavImport.cpp b/app/WavImport.cpp index 379bc5f..56666c8 100644 --- a/app/WavImport.cpp +++ b/app/WavImport.cpp @@ -4,12 +4,15 @@ #include "WavImport.h" #include "Mp3AudioFormat.h" +#include "OperationsLog.h" #include #include #include #include +#include +#include #include namespace loopercat::wavimport @@ -21,14 +24,10 @@ namespace // The pedal takes the file as-is when the core's own upload gate does — // one truth, not a parallel reimplementation of it. - bool pedalAcceptsAsIs(const juce::File& source) + bool pedalAcceptsAsIs(wav::BytesView bytes) { - juce::MemoryBlock raw; - if (!source.loadFileAsData(raw)) - return false; try { - wav::assertUploadable(wav::readWavInfo(wav::BytesView( - static_cast(raw.getData()), raw.getSize()))); + wav::assertUploadable(wav::readWavInfo(bytes)); return true; } catch (const Error&) { return false; @@ -73,14 +72,77 @@ namespace remaining -= n; } } + + // The conversion's file inside its job directory. The name says what the + // file is; the name on the card is decided elsewhere. + constexpr const char* kConvertedFileName = "converted.wav"; + + // JUCE's readers that read a file's samples as they lie, so the width + // they report is a fact about the file. Named by the READER + // (AudioFormatReader::getFormatName — wavFormatName and its siblings in + // juce_audio_formats/codecs), not by the format object that was asked: + // WavAudioFormat hands an Ogg stream inside a WAV to the Ogg reader. A + // JUCE upgrade that renamed one would make its files read as decoded by + // the system, which errs toward the mark. + constexpr const char* kWavReader = "WAV file"; + constexpr const char* kAiffReader = "AIFF file"; + constexpr const char* kFlacReader = "FLAC file"; + constexpr const char* kOggReader = "Ogg-Vorbis file"; + // The words for a system decoder — CoreAudio on macOS, Windows Media on + // Windows — which takes what JUCE's own readers refuse (float64, mu-law, + // AAC) and reports whatever width it chose: nothing about the file. + constexpr const char* kSystemDecoded = "decoded by the system"; + + SourceFormat factsOf(const juce::AudioFormatReader& reader) + { + SourceFormat facts { .sampleRate = int(std::llround(reader.sampleRate)), + .channels = static_cast(reader.numChannels) }; + const juce::String name = reader.getFormatName(); + if (name == Mp3AudioFormat::kReaderName) { + facts.encoding = "MP3"; + } else if (name == kOggReader) { + facts.encoding = "Ogg Vorbis"; + } else if (name == kWavReader || name == kAiffReader || name == kFlacReader) { + facts.encoding = juce::String(int(reader.bitsPerSample)) + "-bit" + + (reader.usesFloatingPointData ? " float" : ""); + facts.factual = true; + } else { + facts.encoding = kSystemDecoded; + } + return facts; + } + + // The words the push report uses for a channel count prepare lets through. + juce::String channelsWord(int channels) { return channels == 1 ? "mono" : "stereo"; } + + // A source the pedal takes as it is: no conversion, no job directory. + Prepared untouched(const juce::File& source, std::optional outcome) + { + Prepared p; + p.file = source; + p.converted = false; + p.normalize = outcome; + return p; + } } // namespace -juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepared& out, +juce::Result prepare(const juce::File& source, const juce::File& importTmp, Prepared& out, const Options& options) { - const bool passesAsIs = pedalAcceptsAsIs(source); + juce::MemoryBlock raw; + if (!source.loadFileAsData(raw)) + return juce::Result::fail("cannot read " + source.getFullPathName()); + const wav::BytesView bytes(static_cast(raw.getData()), raw.getSize()); + // A WAV whose structure is at fault is refused with the fault named, + // never handed to a decoder that would pad a cut data chunk with + // silence or keep one of two data chunks and drop the other (review of + // issue #139). What the gate then refuses is shape — a tag, a rate, a + // width — and that is the converter's job. + if (const auto fault = wav::riffFault(bytes)) + return juce::Result::fail(source.getFileName() + " " + *fault); + const bool passesAsIs = pedalAcceptsAsIs(bytes); if (!options.normalizeTargetLufs.has_value() && passesAsIs) { - out = { source, false, std::nullopt }; + out = untouched(source, std::nullopt); return juce::Result::ok(); } @@ -98,6 +160,7 @@ juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepar return juce::Result::fail(source.getFileName() + " has " + juce::String(channels) + " channels — only mono and stereo can go to the pedal"); + const SourceFormat sourceFormat = factsOf(*reader); // Pass 1 of the opt-in normalization (issue #53): measure, decide, and — // when a pedal-ready file needs nothing — keep the byte-exact promise. @@ -113,7 +176,7 @@ juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepar // not a thing to aim at. Import as-is, report the damage. outcome = NormalizeOutcome { .damaged = true, .wildSamples = meter.wildSamples() }; if (passesAsIs) { - out = { source, false, outcome }; + out = untouched(source, outcome); return juce::Result::ok(); } } else if (!measured.has_value()) { @@ -121,7 +184,7 @@ juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepar // and inventing a gain would be a guess. Import as-is and say so. outcome = NormalizeOutcome {}; if (passesAsIs) { - out = { source, false, outcome }; + out = untouched(source, outcome); return juce::Result::ok(); } } else { @@ -129,7 +192,7 @@ juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepar if (passesAsIs && std::abs(wanted) < loudness::kAlreadyAtTargetLu) { outcome = NormalizeOutcome { .measurable = true, .untouched = true, .measuredLufs = *measured }; - out = { source, false, outcome }; + out = untouched(source, outcome); return juce::Result::ok(); } gainDb = loudness::normalizeGainDb(*measured, target, meter.truePeakDb(), @@ -141,12 +204,17 @@ juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepar } } - const juce::Result dirOk = tempDir.createDirectory(); + // The job's own directory: nothing else writes there, so the file inside + // carries a fixed name and nothing is ever renamed around a clash. The + // name the audio lands under on the card is the caller's decision, made + // at the push (issue #139) — this one is never seen outside this job. + JobDir jobDir(importTmp.getChildFile(juce::Uuid().toString())); + const juce::Result dirOk = jobDir.path().createDirectory(); if (dirOk.failed()) return dirOk; - const juce::File dest = - tempDir.getChildFile(source.getFileNameWithoutExtension() + "-pedal.wav") - .getNonexistentSibling(); + const juce::File dest = jobDir.path().getChildFile(kConvertedFileName); + if (dest.existsAsFile()) + return juce::Result::fail(dest.getFullPathName() + " already exists in a fresh job directory"); auto stream = dest.createOutputStream(); if (stream == nullptr) @@ -191,8 +259,59 @@ juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepar } writer.reset(); // flush before anyone reads the file - out = { dest, true, outcome }; + out.file = dest; + out.converted = true; + // Rebuilt unless there is positive evidence the samples are the source's: + // a reader that reads them as they lie reported the pedal's own shape, + // and no gain went in. A header-only rewrite of such a file is the one + // case that is not a rebuild; anything a decoder made is. + out.rebuilt = !sourceFormat.factual || differsFromTarget(sourceFormat) + || (outcome.has_value() && outcome->measurable && !outcome->untouched); + out.normalize = outcome; + out.sourceFormat = sourceFormat; + out.jobDir = std::move(jobDir); return juce::Result::ok(); } +bool differsFromTarget(const SourceFormat& source) +{ + return source.sampleRate != kTargetSampleRate || source.encoding != kTargetEncoding + || source.channels != kTargetChannels; +} + +void JobDir::release() +{ + if (dir_ == juce::File()) + return; + // import-tmp lives in the app's data home, beside operations.log. + if (!dir_.deleteRecursively()) + oplog::append(dir_.getParentDirectory().getParentDirectory(), + "import-tmp: could not remove " + dir_.getFullPathName() + + " after the job; remove it by hand"); + dir_ = juce::File(); +} + +juce::String describeConversion(const SourceFormat& source) +{ + juce::StringArray was, now; + if (source.sampleRate != kTargetSampleRate) { + was.add(juce::String(source.sampleRate) + " Hz"); + now.add(juce::String(kTargetSampleRate) + " Hz"); + } + if (source.encoding != kTargetEncoding) { + was.add(source.encoding); + now.add(kTargetEncoding); + } + if (source.channels != kTargetChannels) { + was.add(channelsWord(source.channels)); + now.add(channelsWord(kTargetChannels)); + } + if (was.isEmpty()) + return {}; + // The same arrow describeNormalize draws, so a note carrying both + // sentences reads with one. + return was.joinIntoString(", ") + juce::String::fromUTF8(" \xe2\x86\x92 ") + + now.joinIntoString(", "); +} + } // namespace loopercat::wavimport diff --git a/app/WavImport.h b/app/WavImport.h index ba9a5af..8dc212a 100644 --- a/app/WavImport.h +++ b/app/WavImport.h @@ -7,6 +7,7 @@ #include #include +#include //============================================================================== // loopercat::wavimport — make a user's audio file pedal-acceptable (issue @@ -15,7 +16,8 @@ // 48 kHz, mono, AIFF. A file the pedal already accepts passes through // UNTOUCHED (the byte-surgery invariant extends to uploads); everything // else JUCE can read is rewritten as 44.1 kHz stereo float32 — the pedal's -// own native recording format — into a temp file the caller owns. +// own native recording format — into a temp directory of its own under the +// caller's import-tmp, which dies with the Prepared that owns it. // // The one exception is opt-in (issue #53): with a normalization target set, // loudness is measured per ITU-R BS.1770-4 over the exact post-resample @@ -54,15 +56,96 @@ struct NormalizeOutcome { std::int64_t wildSamples = 0; // how many, for the report }; +// What the audio was before the rebuild (issue #139) — the facts the push +// report states, against the fixed target of 44100 Hz, 32-bit float, stereo. +struct SourceFormat { + int sampleRate = 0; // Hz + // "24-bit", "32-bit float" — the file's own sample format, when a reader + // that reads samples as they lie said so; the codec ("MP3", "Ogg Vorbis") + // for a bitstream, whose width is the decoder's choice; "decoded by the + // system" for what the OS decoded, which reports whatever it chose. + juce::String encoding; + int channels = 0; // 1 or 2; prepare refuses the rest + // true: the reader reads the file's samples as they lie (JUCE's WAV, AIFF + // and FLAC readers), so `encoding` is a fact about the file and equal + // facts mean equal samples. false: a decoder made the samples, and + // nothing here proves them the file's — such a source is rebuilt however + // its facts read. + bool factual = false; +}; + +// The pedal's side of the sentence; a source that matches it on every point +// gets no sentence at all. +inline constexpr int kTargetSampleRate = 44100; +inline constexpr const char* kTargetEncoding = "32-bit float"; +inline constexpr int kTargetChannels = 2; + +// Whether the samples had to change shape to become the pedal's: a different +// rate, sample format or channel count. A source that matches on all three +// was only repacked — a WAVE_FORMAT_EXTENSIBLE float export the pedal's +// gate does not read, say — and its samples are the player's own. +bool differsFromTarget(const SourceFormat& source); + +// "48000 Hz, 24-bit, mono → 44100 Hz, 32-bit float, stereo", saying only +// what changed: a 44.1 kHz 24-bit stereo file reads "24-bit → 32-bit float", +// and a pedal-ready file that was rewritten for its loudness alone reads as +// nothing — "32-bit float → 32-bit float" is not a change anyone needs told. +juce::String describeConversion(const SourceFormat& source); + +// The conversion's own directory under import-tmp: one per job, so two +// uploads of files with the same stem in one queue never meet, and gone with +// the Prepared that owns it — after the push succeeded and after it threw +// alike. Empty until a conversion needs one. +class JobDir +{ +public: + JobDir() = default; + explicit JobDir(juce::File dir) : dir_(std::move(dir)) {} + ~JobDir() { release(); } + JobDir(JobDir&& other) noexcept : dir_(std::exchange(other.dir_, juce::File())) {} + JobDir& operator=(JobDir&& other) noexcept + { + if (this != &other) { + release(); + dir_ = std::exchange(other.dir_, juce::File()); + } + return *this; + } + JobDir(const JobDir&) = delete; + JobDir& operator=(const JobDir&) = delete; + + const juce::File& path() const { return dir_; } + +private: + // A delete that fails is written to the operations log beside the + // directory's data home; nothing sweeps import-tmp yet, so the line is + // the only trace a leftover has. + void release(); + juce::File dir_; +}; + struct Prepared { juce::File file; // hand this to commands::push - bool converted = false; // true: `file` is a temp conversion — delete it after the push + bool converted = false; // true: `file` is a conversion in `jobDir`, not the source + // true: the samples are not the source's — the shape changed + // (differsFromTarget) or a gain went in. A pedal-ready file rewritten for + // its header alone is converted but not rebuilt; the mark keys on this. + bool rebuilt = false; std::optional normalize; // engaged iff Options asked for it + std::optional sourceFormat; // what the source was; present iff converted + JobDir jobDir; // the conversion's directory; its lifetime is the file's }; -// Fails when the source is not audio JUCE can read, or has more than two -// channels — a surround downmix is a creative decision, not a default. -juce::Result prepare(const juce::File& source, const juce::File& tempDir, Prepared& out, +// Fails when the source is not audio JUCE can read, has more than two +// channels — a surround downmix is a creative decision, not a default — or +// is a WAV whose structure is at fault (wav::riffFault: cut short, never +// finalised, two data chunks): a decoder would pad, drop or guess, and the +// pedal would get audio that is not the file's under the player's own name. +// A header the core's gate does not read (an extensible tag) is a shape, not +// a fault: the converter reads it. +// `importTmp` is the app's import-tmp root; a conversion gets a directory of +// its own under it. +juce::Result prepare(const juce::File& source, const juce::File& importTmp, Prepared& out, const Options& options); } // namespace loopercat::wavimport diff --git a/app/history/SlotRows.h b/app/history/SlotRows.h index 5b26c0a..8278f51 100644 --- a/app/history/SlotRows.h +++ b/app/history/SlotRows.h @@ -70,6 +70,7 @@ inline Row one(const HistoryStore::TimelineEntry& entry, bool newest) .swappedWith = entry.swappedWith, .takeName = entry.takeName, .take = take, + .status = entry.status, .note = entry.note }); row.playable = entry.takeKept; row.restorable = !newest && entry.afterBody.has_value() && (!hasTake || entry.takeKept); @@ -227,7 +228,7 @@ inline std::vector forCard(const std::vector& // Nothing recorded about any slot: the words story::tell has for // the operation itself, including a kind this build has no words for. const story::Line bare = story::tell({ .kind = entry.kind, .take = story::Take::none, - .note = entry.note }); + .status = entry.status, .note = entry.note }); row.action = bare.action; row.detail = bare.detail.empty() ? entry.note : bare.detail; // a failed op's reason } diff --git a/app/history/SlotStory.h b/app/history/SlotStory.h index fcff6ae..6a90673 100644 --- a/app/history/SlotStory.h +++ b/app/history/SlotStory.h @@ -54,6 +54,10 @@ struct Facts { std::optional swappedWith; std::string takeName; // the take the slot held after the operation Take take = Take::none; + // The operation's status as the store records it ("done", "failed", + // "interrupted"): a note is the job's own words only when the job + // finished — a failed operation's note is its error. + std::string status; std::string note; }; @@ -98,6 +102,26 @@ inline std::string trimmedName(const std::string& body) return name; } +// The sentences of a job's note other than `fact`: a push's note is +// "; ; ", each present only +// when it has something to say, and the fact has its own place in the line. +inline std::string sentencesBesides(const std::string& note, std::string_view fact) +{ + constexpr std::string_view kJoin = "; "; + std::string out; + std::size_t at = 0; + while (at <= note.size()) { + const std::size_t end = note.find(kJoin, at); + const std::string part = note.substr(at, end == std::string::npos ? std::string::npos : end - at); + if (!part.empty() && part != fact) + out += (out.empty() ? "" : std::string(kJoin)) + part; + if (end == std::string::npos) + break; + at = end + kJoin.size(); + } + return out; +} + inline Line tell(const Facts& facts) { Line line; @@ -130,6 +154,14 @@ inline Line tell(const Facts& facts) (void) was; line.detail += " - " + minutes(now) + " - " + bpm(tempoTenths(*facts.afterBody)); } + // What the job said about the take on its way in — the rebuild's + // facts, the normalization's — after the numbers (issue #139): the + // row and the toast tell the same story. Only for a job that + // finished: a failed one's note is its error, and the row's state + // already says it failed. + if (facts.status == "done") + if (const std::string said = sentencesBesides(facts.note, kNoteLengthReplaced); !said.empty()) + line.detail += (line.detail.empty() ? "" : " - ") + said; sayNoteLength(); } else if (facts.kind == "trim") { line.action = "Trimmed"; diff --git a/core/include/loopercat/Commands.hpp b/core/include/loopercat/Commands.hpp index f418110..6bebc25 100644 --- a/core/include/loopercat/Commands.hpp +++ b/core/include/loopercat/Commands.hpp @@ -17,6 +17,7 @@ #include "DeviceProfile.hpp" #include "Downmix.hpp" #include "Error.hpp" +#include "FatName.hpp" #include "Loudness.hpp" #include "Normalize.hpp" #include "Params.hpp" @@ -25,6 +26,7 @@ #include "Volume.hpp" #include "Wav.hpp" +#include #include #include #include @@ -32,6 +34,7 @@ #include #include #include +#include #include namespace loopercat::commands { @@ -560,12 +563,64 @@ inline WriteResult setTempo(const fs::path& volume, int slot, long long tempoTen struct PushOptions { std::optional name; // also rename the slot + // The file's name on the card (issue #139). Absent: the name of the file + // push is handed, as before. Present: exactly this — the caller decides + // what a converted upload is called, instead of a temp file's name + // travelling to the card by accident. Checked by assertLandedName before + // a byte moves. + std::optional landedName; bool oneShot = false; bool writeConfig = true; // false = drop the file only, let the pedal index it on boot bool force = false; // replace existing slot audio (it goes to write.archive first) WriteOptions write; }; +// A landed name is a bare file name with a .wav extension and something in +// front of it, that every host of the card can create and the card's own +// sweep leaves alone — checked here, before a byte moves, because push +// archives and removes the old take before it writes the new one, and a +// name the card refuses at the write would leave the slot empty with a +// config that says it holds audio (issue #139, review). The rules are +// FatName.hpp's: no separator in either direction (the card is FAT and is +// read on Windows too), none of FAT's reserved characters, at most 238 +// UTF-16 units (what Windows's MAX_PATH leaves under the card folder — FAT's +// own 255 is not the binding limit), no Windows device name; plus the card's own: no name the +// junk sweep deletes (volume::isJunkName — "._take.wav" would be reported +// as pushed and swept at the next sweep), no name starting with a dot (a +// hidden file on every host). The extension's case is not checked: the +// pedal's own recordings wear uppercase .WAV (the pedal-technical names +// Wav.hpp recognises) and FAT does not tell the two apart. An upload's +// extension changes on purpose (issue #139: song.mp3 lands as +// song-pedal.wav), so a name that kept its source's extension would lie +// about the bytes under it. +inline void assertLandedName(const std::string& name) +{ + if (name.find_first_of("/\\") != std::string::npos) + throw Error("push: the name on the card must be a bare file name, not \"" + name + "\""); + if (const auto c = fatname::forbiddenCharacter(name)) + throw Error("push: the name on the card cannot contain " + fatname::describeCharacter(*c) + + " — a file name on the card cannot hold it"); + constexpr std::string_view kWav = ".wav"; + std::string tail = name.size() >= kWav.size() ? name.substr(name.size() - kWav.size()) : name; + for (char& c : tail) + c = static_cast(std::tolower(static_cast(c))); + if (tail != kWav) + throw Error("push: the name on the card must end in .wav, not \"" + name + "\""); + if (name.size() == kWav.size()) + throw Error("push: the name on the card needs a name in front of .wav"); + if (volume::isJunkName(name)) + throw Error("push: \"" + name + "\" is a name the card sweep deletes"); + if (name.front() == '.') + throw Error("push: the name on the card cannot start with a dot — \"" + name + + "\" would be a hidden file"); + if (fatname::isDeviceName(name)) + throw Error("push: \"" + name + "\" is a name Windows reserves for a device"); + if (const std::size_t units = fatname::utf16Units(name); units > fatname::kMaxLandedUnits) + throw Error("push: the name on the card is " + std::to_string(units) + + " characters long; a file name on the card holds at most " + + std::to_string(fatname::kMaxLandedUnits)); +} + struct PushResult { wav::Info info; fs::path dest; @@ -598,6 +653,8 @@ inline PushResult push(const fs::path& volume, const fs::path& wavPath, int slot if (options.name) rc0::encodeName(*options.name); // validates; applied in the document below + if (options.landedName) + assertLandedName(*options.landedName); std::optional slotParams; std::string newDocument; @@ -652,8 +709,9 @@ inline PushResult push(const fs::path& volume, const fs::path& wavPath, int slot fs::create_directories(dir, ec); if (ec) throw Error("cannot create " + dir.string()); - PushResult result { info, dir / wavPath.filename(), false, noteLengthReplaced, slotParams, - {}, std::nullopt }; + PushResult result { info, + dir / (options.landedName ? fs::path(*options.landedName) : wavPath.filename()), + false, noteLengthReplaced, slotParams, {}, std::nullopt }; for (const auto& old : existing) { archiveTake(options.write, "push", slot, old, readFileBytes(dir / old)); result.archived.push_back(old); diff --git a/core/include/loopercat/FatName.hpp b/core/include/loopercat/FatName.hpp new file mode 100644 index 0000000..76311fc --- /dev/null +++ b/core/include/loopercat/FatName.hpp @@ -0,0 +1,99 @@ +// Copyright (c) 2026 Darwin's Cat. Part of Looper Cat — see LICENSE. +// SPDX-License-Identifier: AGPL-3.0-or-later + +#pragma once + +#include +#include +#include +#include +#include + +namespace loopercat::fatname { + +// What a file name on the card may be. The card is FAT and is read by every +// host OS the pedal meets, so the rules are FAT's long-name rules plus the +// names Windows refuses to open. One place for them: the core checks a name +// before a byte moves (Commands.hpp, push), the app checks a mark before it +// is stored (app/UploadMark.h), and both must agree. + +// Characters a FAT long file name cannot hold: the reserved nine (Microsoft, +// "Naming Files, Paths, and Namespaces"), plus the control range 0x00-0x1F +// from the same page. +inline constexpr std::string_view kReserved = "/\\:*?\"<>|"; + +inline bool isControl(char c) { return static_cast(c) < 0x20; } + +// A FAT long name holds at most 255 UTF-16 units (Microsoft FAT32 File +// System Specification, long directory entries: 13 units per entry, 20 +// entries at most, 255 after the terminator). +inline constexpr std::size_t kMaxUnits = 255; + +// The card is read on Windows too, where a path is capped at MAX_PATH — 260 +// characters, the terminator included — unless the host opted into long +// paths (Windows 10 1607+: a registry switch AND an application manifest; +// off by default, and neither JUCE nor this app declares one). The card +// folder "E:\ROLAND\WAVE\001_1\" is 21 characters, so a name under it may +// be 260 - 1 - 21 = 238 units before the write fails there — after push has +// archived and removed the old take (review of issue #139). The cap holds on +// every platform: the same card meets every host. +inline constexpr std::size_t kWindowsMaxPath = 260; +inline constexpr std::size_t kCardFolderChars = 21; +inline constexpr std::size_t kMaxLandedUnits = kWindowsMaxPath - 1 - kCardFolderChars; +static_assert(kMaxLandedUnits == 238); +static_assert(kMaxLandedUnits <= kMaxUnits); + +// The first character `text` holds that a FAT name cannot; empty when none. +inline std::optional forbiddenCharacter(std::string_view text) +{ + for (const char c : text) + if (kReserved.find(c) != std::string_view::npos || isControl(c)) + return c; + return std::nullopt; +} + +// How a refusal names such a character: the character itself, quoted, or +// "a control character" for one that has no face. +inline std::string describeCharacter(char c) +{ + return isControl(c) ? std::string("a control character") : std::string("\"") + c + "\""; +} + +// The UTF-16 units of a UTF-8 string — what the FAT limit counts. A lead +// byte starts a code point (one unit); a 4-byte lead starts a supplementary +// one (a surrogate pair, two units); continuation bytes add nothing. The +// string is taken as well-formed UTF-8: every name here comes from a path the +// host OS produced or from a juce::String. +inline std::size_t utf16Units(std::string_view utf8) +{ + std::size_t units = 0; + for (const char ch : utf8) { + const auto b = static_cast(ch); + if ((b & 0xC0) == 0x80) + continue; + units += (b & 0xF8) == 0xF0 ? 2 : 1; + } + return units; +} + +// The names Windows reserves for devices — CON, PRN, AUX, NUL, COM1-9, +// LPT1-9, and COM/LPT with the superscript digits ¹ ² ³ (U+00B9, U+00B2, +// U+00B3), which Windows reserves alongside 1-3 — judged on the part before +// the FIRST dot, case-insensitively, which is Windows's own rule: "con.wav" +// is as unopenable as "CON". +inline bool isDeviceName(std::string_view name) +{ + std::string head(name.substr(0, name.find('.'))); + for (char& c : head) // toupper leaves a UTF-8 byte alone, so the superscripts survive + c = static_cast(std::toupper(static_cast(c))); + if (head == "CON" || head == "PRN" || head == "AUX" || head == "NUL") + return true; + if (head.size() < 4 || !(head.starts_with("COM") || head.starts_with("LPT"))) + return false; + const std::string_view digit = std::string_view(head).substr(3); + if (digit.size() == 1) + return digit[0] >= '1' && digit[0] <= '9'; + return digit == "\xc2\xb9" || digit == "\xc2\xb2" || digit == "\xc2\xb3"; // ¹ ² ³ +} + +} // namespace loopercat::fatname diff --git a/core/include/loopercat/Wav.hpp b/core/include/loopercat/Wav.hpp index 9d6f6b1..ff7c42a 100644 --- a/core/include/loopercat/Wav.hpp +++ b/core/include/loopercat/Wav.hpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -202,6 +203,61 @@ inline Bytes trimmed(BytesView data, std::int64_t startFrame, std::int64_t endFr return out; } +// A fault in a RIFF/WAVE buffer's STRUCTURE — one no reader can make audio +// of without inventing some — as a clause to put after the file's name, for +// an importer to refuse with (issue #139, review). Empty for a sound +// structure, and for a buffer that is not RIFF/WAVE at all: that is +// something else's problem. The fmt chunk's fields are not structure: an +// extensible or 24-bit header is a shape the converter reads, and +// readWavInfo plus the upload gate decide about it. +// +// Chunks past the end are told apart on purpose: a data chunk short of its +// claim is a recording cut off, named by how much; a data size of +// 0xFFFFFFFF is a header a writer never finalised; a metadata chunk cut off +// AFTER a complete data chunk — a LIST trailer — costs no sample and is no +// fault here (readWavInfo still calls the file truncated, so it goes +// through the converter, samples intact). Two data chunks are refused +// because a reader keeps one and drops the other without a word. +inline std::optional riffFault(BytesView data) +{ + if (data.size() < 12 || !detail::chunkIdIs(data, 0, "RIFF") + || !detail::chunkIdIs(data, 8, "WAVE")) + return std::nullopt; + constexpr std::int64_t kUnfinalised = 0xFFFFFFFF; + const auto size = static_cast(data.size()); + int fmtChunks = 0, dataChunks = 0; + std::int64_t offset = 12; + while (offset + 8 <= size) { + const auto o = static_cast(offset); + const std::int64_t chunkSize = detail::u32(data, o + 4); + const bool isData = detail::chunkIdIs(data, o, "data"); + const bool isFmt = detail::chunkIdIs(data, o, "fmt "); + if (isData && chunkSize == kUnfinalised) + return "was never finalised: its data chunk's size is still unset"; + if (offset + 8 + chunkSize > size) { + if (isData) + return "is cut short: its data chunk claims " + + std::to_string(offset + 8 + chunkSize - size) + + " bytes more than the file holds"; + break; // a metadata chunk cut off: what stands before it is whole + } + if (isFmt) { + if (++fmtChunks > 1) + return "has two fmt chunks"; + if (chunkSize < 16) + return "has a malformed fmt chunk of " + std::to_string(chunkSize) + " bytes"; + } + if (isData && ++dataChunks > 1) + return "has two data chunks"; + offset += 8 + chunkSize + (chunkSize % 2); + } + if (fmtChunks == 0) + return "has no fmt chunk"; + if (dataChunks == 0) + return "has no data chunk"; + return std::nullopt; +} + // Rewrite a WAV into the pedal's own canonical shape: RIFF + fmt + data, // nothing else. The RC-5 rewrites non-canonical files (DAW metadata chunks) // during boot-time indexing; handing it an already-canonical file means it diff --git a/docs/guide.md b/docs/guide.md index 91843d9..5f6ad9d 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -104,6 +104,9 @@ The gear in the top row opens the app's own settings: - **Audio** — which interface and outputs the preview plays through. - **Columns** — which behaviour lamps the slot table shows. The pedal's own facts (name, duration, bars, tempo, file) are always there. +- **Import** — what happens to a file on its way to the pedal: normalization + (see [Normalize](#normalize)) and the mark a converted upload carries (see + [Moving audio](#moving-audio)). ## Editing a slot @@ -237,6 +240,22 @@ A few things worth knowing: seam); when replacing, the old loop is kept in the history. - **Pull to folder…** — copy a loop off the pedal as a standard WAV. +A file the pedal takes as it is (44.1 kHz, stereo, 32-bit float WAV) lands +under its own name, byte for byte. One that had to be rebuilt — a 48 kHz or +24-bit export, a mono take, an MP3, or a file normalization rewrote — lands +as `-pedal.wav`: `song.mp3` becomes `song-pedal.wav`. The mark is +there on purpose. The bytes in that slot are not the bytes of your file (they +were resampled, spread to two channels, re-quantised, maybe gain-adjusted), and +a take pulled back a month later should say so at a glance. The push toast and +the history row spell out what changed, e.g. `48000 Hz, 24-bit, mono → +44100 Hz, 32-bit float, stereo`. A float export that differs only in its +header (the WAVE_FORMAT_EXTENSIBLE shape DAWs write) is repacked, not rebuilt: +it lands as `.wav`, every sample intact, with no mark. The suffix +is yours: Settings → **Import** → +*Mark uploads converted for the pedal with*. Clear it and converted uploads +land under their own name (with a `.wav` extension — the audio inside is a +WAV, whatever the source was). + ## Swap Drag a row by its grip onto another row to swap the two slots — names, audio, diff --git a/tests/commands_tests.cpp b/tests/commands_tests.cpp index ffc0d30..0e92145 100644 --- a/tests/commands_tests.cpp +++ b/tests/commands_tests.cpp @@ -1123,6 +1123,129 @@ int main() == pushed); } + // --- push lands the file under the name it is told (issue #139) --- + // + // The caller decides what a converted upload is called; push writes + // exactly that, and refuses a name that is not a bare .wav file name + // before a byte moves — the slot and the archive stay as they were. + { + TempDir tmp; + const fs::path volume = makePedal(tmp.path); + const auto wavBytes = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 1323000 }); + // The file push is handed carries a working name that must NOT reach the card. + const fs::path source = tmp.path / "converted.wav"; + commands::writeFileBytes(source, std::string_view(reinterpret_cast(wavBytes.data()), + wavBytes.size())); + + const auto landed = commands::push(volume, source, 9, + { .landedName = "song-pedal.wav", .write = writeOpts(tmp.path) }); + CHECK_EQ(landed.dest.filename().string(), "song-pedal.wav"); + CHECK(fs::exists(landed.dest)); + CHECK(volume::listSlotWavs(volume, 9) == std::vector { "song-pedal.wav" }); + CHECK(!fs::exists(landed.dest.parent_path() / "converted.wav")); + + // The pedal's own case for the extension is a name too. + const auto upper = commands::push(volume, source, 10, + { .landedName = "TAKE.WAV", .write = writeOpts(tmp.path) }); + CHECK_EQ(upper.dest.filename().string(), "TAKE.WAV"); + + // Absent: the name of the file push was handed, as before. + const auto plain = commands::push(volume, source, 11, { .write = writeOpts(tmp.path) }); + CHECK_EQ(plain.dest.filename().string(), "converted.wav"); + + // Refusals, each before any write: an empty slot stays empty, no + // folder appears, and the volume is byte-identical. + const auto before = volumeBytes(volume); + for (const std::string bad : { "sub/song.wav", "sub\\song.wav", "/song.wav", "song.wav/" }) + CHECK_THROWS(commands::push(volume, source, 12, + { .landedName = bad, .write = writeOpts(tmp.path) }), + "bare file name"); + for (const std::string bad : { "song.mp3", "song", "song.wav.bak", "wav", "" }) + CHECK_THROWS(commands::push(volume, source, 12, + { .landedName = bad, .write = writeOpts(tmp.path) }), + "end in .wav"); + CHECK_THROWS(commands::push(volume, source, 12, + { .landedName = ".wav", .write = writeOpts(tmp.path) }), + "name in front of .wav"); + CHECK(volumeBytes(volume) == before); + CHECK(!fs::exists(volume::wavDir(volume, 12))); + + // On an occupied slot with force: the refusal comes before the + // archive is handed anything and before the old take moves. + const commands::WriteOptions replacing = writeOpts(tmp.path, "op-bad-name"); + CHECK_THROWS(commands::push(volume, source, 9, + { .landedName = "song.mp3", .force = true, .write = replacing }), + "end in .wav"); + CHECK(volume::listSlotWavs(volume, 9) == std::vector { "song-pedal.wav" }); + CHECK(!fs::exists(tmp.path / "archive" / replacing.opId)); + CHECK(volumeBytes(volume) == before); + + // The review's P0: a name longer than the card can take used to pass + // the up-front check, so push archived and removed the old take and + // then failed at the write — an empty slot folder under a config that + // says it holds audio. The cap is 238 UTF-16 units: what Windows's + // MAX_PATH (260, terminator included) leaves under the 21-character + // card folder — FAT's own 255 is not the binding limit. A + // 252-character stem plus .wav: refused before the archive is + // touched, the old take stays, the card is byte-identical. + const commands::WriteOptions longName = writeOpts(tmp.path, "op-long-name"); + CHECK_THROWS(commands::push(volume, source, 9, + { .landedName = std::string(252, 'n') + ".wav", .force = true, + .write = longName }), + "at most 238"); + CHECK(volume::listSlotWavs(volume, 9) == std::vector { "song-pedal.wav" }); + CHECK(!fs::exists(tmp.path / "archive" / longName.opId)); + CHECK(volumeBytes(volume) == before); + // The name that passed every check and still failed on Windows: + // 239 units. 238 passes, 239 is refused. The file itself is not + // written here, where a host temp path plus the name would exceed + // what a Windows runner allows. + commands::assertLandedName(std::string(234, 'n') + ".wav"); + CHECK_THROWS(commands::assertLandedName(std::string(235, 'n') + ".wav"), "239"); + CHECK_THROWS(commands::assertLandedName(std::string(251, 'n') + ".wav"), "at most 238"); + // units, not bytes: 200 "é" are 400 bytes and 200 units + std::string accented; + for (int i = 0; i < 200; ++i) + accented += "\xc3\xa9"; + commands::assertLandedName(accented + ".wav"); + ++testkit::checksRun; // the two names above were accepted + + // Names every host of the card can create and the card's own sweep + // leaves alone — anything else is refused up front, with the slot + // and the card as they were. + for (const auto& [bad, why] : std::vector> { + { "ta:ke.wav", "cannot contain \":\"" }, + { "take?.wav", "cannot contain \"?\"" }, + { ".wav", "cannot contain \"<\"" }, + { "take|2.wav", "cannot contain \"|\"" }, + { "ta\"ke.wav", "cannot contain \"\"\"" }, + { "take*.wav", "cannot contain \"*\"" }, + { std::string("take\x01.wav"), "control character" }, + { std::string("take\t.wav"), "control character" }, + { "._take.wav", "card sweep deletes" }, + { ".take.wav", "start with a dot" }, + { "con.wav", "Windows reserves" }, + { "CON.wav", "Windows reserves" }, + { "LPT1.wav", "Windows reserves" }, + { "Com9.take.wav", "Windows reserves" }, + { "nul.wav", "Windows reserves" } }) { + CHECK_THROWS(commands::push(volume, source, 12, { .landedName = bad, .write = writeOpts(tmp.path) }), + why); + CHECK_THROWS(commands::push(volume, source, 9, + { .landedName = bad, .force = true, .write = writeOpts(tmp.path) }), + why); + } + CHECK(volume::listSlotWavs(volume, 9) == std::vector { "song-pedal.wav" }); + CHECK(!fs::exists(volume::wavDir(volume, 12))); + CHECK(volumeBytes(volume) == before); + // ...and what only looks reserved is not: a word that starts with a + // device name, COM10, a mark after the stem + for (const char* fine : { "console.wav", "COM10.wav", "aux-pedal.wav", "My Song (take 2).wav", + "caf\xc3\xa9 \xe2\x80\x93 live.wav" }) + commands::assertLandedName(fine); + ++testkit::checksRun; // all accepted + } + // --- push failure leaves the volume byte-identical --- { diff --git a/tests/fat_name_tests.cpp b/tests/fat_name_tests.cpp new file mode 100644 index 0000000..43fd7c2 --- /dev/null +++ b/tests/fat_name_tests.cpp @@ -0,0 +1,88 @@ +// Copyright (c) 2026 Darwin's Cat. Part of Looper Cat — see LICENSE. +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// What a file name on the card may be (FatName.hpp), from FAT's own rules +// and Windows's: the reserved characters and the control range, the 255 +// UTF-16 units a long name holds — counted as units, not bytes — and the +// device names Windows will not open, judged the way Windows judges them. + +#include "support.hpp" + +#include + +#include + +using namespace loopercat; + +int main() +{ + // --- the limits: FAT's 255 units, and the 238 Windows's MAX_PATH leaves under the card folder --- + CHECK_EQ(fatname::kMaxUnits, std::size_t { 255 }); + CHECK_EQ(fatname::kWindowsMaxPath, std::size_t { 260 }); + CHECK_EQ(fatname::kCardFolderChars, std::string("E:\\ROLAND\\WAVE\\001_1\\").size()); + CHECK_EQ(fatname::kMaxLandedUnits, std::size_t { 238 }); + CHECK_EQ(fatname::kMaxLandedUnits, fatname::kWindowsMaxPath - 1 - fatname::kCardFolderChars); + + // --- units, not bytes: ASCII one each, BMP one each, astral two --- + CHECK_EQ(fatname::utf16Units(""), std::size_t { 0 }); + CHECK_EQ(fatname::utf16Units("abc.wav"), std::size_t { 7 }); + CHECK_EQ(fatname::utf16Units("caf\xc3\xa9"), std::size_t { 4 }); // é: 2 bytes, 1 unit + CHECK_EQ(fatname::utf16Units("\xe2\x86\x92"), std::size_t { 1 }); // →: 3 bytes, 1 unit + CHECK_EQ(fatname::utf16Units("\xf0\x9f\x98\x80"), std::size_t { 2 }); // 😀: 4 bytes, 2 units + CHECK_EQ(fatname::utf16Units("a\xf0\x9f\x98\x80z"), std::size_t { 4 }); + CHECK_EQ(fatname::utf16Units(std::string(255, 'a')), std::size_t { 255 }); + CHECK_EQ(fatname::utf16Units(std::string(256, 'a')), std::size_t { 256 }); + + // --- the reserved nine and the control range are refused by name --- + CHECK_EQ(std::string(fatname::kReserved), std::string("/\\:*?\"<>|")); + for (const char c : std::string("/\\:*?\"<>|")) { + const auto found = fatname::forbiddenCharacter(std::string("ta") + c + "ke"); + CHECK(found.has_value()); + if (found.has_value()) { + CHECK_EQ(*found, c); + CHECK_EQ(fatname::describeCharacter(*found), std::string("\"") + c + "\""); + } + } + for (const char c : { '\0', '\t', '\n', '\x1f' }) { + const auto found = fatname::forbiddenCharacter(std::string("ta") + c + "ke"); + CHECK(found.has_value()); + if (found.has_value()) { + CHECK(fatname::isControl(*found)); + CHECK_EQ(fatname::describeCharacter(*found), std::string("a control character")); + } + } + // the first offender is the one named + { + const auto found = fatname::forbiddenCharacter("a?b:c"); + CHECK(found.has_value() && *found == '?'); + } + + // --- what a FAT name may hold is not refused: spaces, dots, unicode, 0x7f --- + CHECK(!fatname::forbiddenCharacter("").has_value()); + CHECK(!fatname::forbiddenCharacter("My Song (take 2).wav").has_value()); + CHECK(!fatname::forbiddenCharacter("caf\xc3\xa9 \xe2\x80\x93 live.wav").has_value()); + CHECK(!fatname::forbiddenCharacter("v1.2 mix-pedal.wav").has_value()); + CHECK(!fatname::forbiddenCharacter(std::string("a\x7f", 2)).has_value()); + + // --- Windows device names, before the first dot, in any case --- + for (const char* reserved : { "CON", "con", "Con.wav", "PRN.wav", "aux.wav", "NUL.wav", + "COM1", "com9.wav", "LPT1.wav", "lpt9", "Com1.take.wav" }) + CHECK(fatname::isDeviceName(reserved)); + for (const char* plain : { "", "CONSOLE.wav", "COM10.wav", "COM0.wav", "LPT.wav", "COM.wav", + "aux-pedal.wav", "nul1.wav", "song.wav", ".wav" }) + CHECK(!fatname::isDeviceName(plain)); + // the superscript digits Windows reserves alongside 1-3: all twelve, in any case, with and without an extension + for (const char* sup : { "\xc2\xb9", "\xc2\xb2", "\xc2\xb3" }) { + for (const char* device : { "COM", "LPT", "com", "lpt" }) { + CHECK(fatname::isDeviceName(std::string(device) + sup)); + CHECK(fatname::isDeviceName(std::string(device) + sup + ".wav")); + } + } + // ...and only those: a degree sign (C2 B0), a superscript four (E2 81 B4), a digit followed by more + CHECK(!fatname::isDeviceName("COM\xc2\xb0.wav")); + CHECK(!fatname::isDeviceName("COM\xe2\x81\xb4.wav")); + CHECK(!fatname::isDeviceName("COM\xc2\xb9x.wav")); + CHECK(!fatname::isDeviceName("COM1x.wav")); + + return testkit::summary("fat_name"); +} diff --git a/tests/import_prefs_tests.cpp b/tests/import_prefs_tests.cpp new file mode 100644 index 0000000..9548385 --- /dev/null +++ b/tests/import_prefs_tests.cpp @@ -0,0 +1,146 @@ +// Copyright (c) 2026 Darwin's Cat. Part of Looper Cat — see LICENSE. +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// The upload preferences in the app's own PropertiesFile (issues #53, #139), +// against what Settings promises: a file nobody has touched reads as the +// shipped defaults, what is written is what is read back by a fresh open of +// the same file — a mark cleared to nothing included — and the keys sit +// beside each other under their own names, so a hand-edited settings file +// reads the way it looks. + +#include "support.hpp" + +#include "../app/ImportPrefs.h" + +#include + +#include +#include +#include + +using namespace loopercat; + +namespace +{ + +// A PropertiesFile in `dir`, the way AppSettings opens it. Every open is a +// fresh object, so a value that survives is one that reached the disk. +std::unique_ptr open(const juce::File& dir) +{ + juce::PropertiesFile::Options options; + options.applicationName = "LooperCat"; + options.filenameSuffix = ".settings"; + options.folderName = dir.getFullPathName(); + options.osxLibrarySubFolder = "Application Support"; + return std::make_unique(options); +} + +// A target that went through the file as text comes back as the same number; +// the comparison allows for nothing but the representation. +bool nearly(double actual, double expected) { return std::abs(actual - expected) < 1.0e-9; } + +} // namespace + +int main() +{ + juce::ScopedJuceInitialiser_GUI juceInit; + const juce::File work = juce::File::getSpecialLocation(juce::File::tempDirectory) + .getChildFile("import_prefs_tests") + .getNonexistentSibling(); + work.createDirectory(); + + // --- a file nobody touched reads as the shipped defaults --- + { + auto file = open(work); + const ImportPrefs prefs = importprefs::read(*file); + CHECK(!prefs.normalizeOnUpload); + CHECK(nearly(prefs.targetLufs, -18.0)); + CHECK_EQ(prefs.convertedMark.toStdString(), std::string("-pedal")); + CHECK(!file->containsKey(importprefs::kConvertedMarkKey)); // reading wrote nothing + } + + // --- what is written is what a fresh open reads back --- + { + { + auto file = open(work); + importprefs::write(*file, { .normalizeOnUpload = true, + .targetLufs = -16.5, + .convertedMark = "(pedal) v2" }); + } // closed: the next open reads the disk, not this object + auto file = open(work); + const ImportPrefs prefs = importprefs::read(*file); + CHECK(prefs.normalizeOnUpload); + CHECK(nearly(prefs.targetLufs, -16.5)); + CHECK_EQ(prefs.convertedMark.toStdString(), std::string("(pedal) v2")); // the space inside survives + } + + // --- a mark cleared to nothing stays nothing: "" is a value, not an absence --- + { + { + auto file = open(work); + ImportPrefs prefs = importprefs::read(*file); + prefs.convertedMark = ""; + importprefs::write(*file, prefs); + } + auto file = open(work); + const ImportPrefs prefs = importprefs::read(*file); + CHECK(file->containsKey(importprefs::kConvertedMarkKey)); + CHECK_EQ(prefs.convertedMark.toStdString(), std::string("")); + CHECK(prefs.normalizeOnUpload); // the other two keys were not touched + CHECK(nearly(prefs.targetLufs, -16.5)); + } + + // --- a mark of only whitespace reads as empty; every other mark reads as written: + // one rule (markAsTyped), shared with the Settings field, and nothing else rewritten --- + { + { + auto file = open(work); + file->setValue(importprefs::kConvertedMarkKey, " "); + file->saveIfNeeded(); + } + auto file = open(work); + CHECK_EQ(importprefs::read(*file).convertedMark.toStdString(), std::string("")); + file->setValue(importprefs::kConvertedMarkKey, "\t \n"); + CHECK_EQ(importprefs::read(*file).convertedMark.toStdString(), std::string("")); + file->setValue(importprefs::kConvertedMarkKey, " -live "); // spaces around stay: as written + CHECK_EQ(importprefs::read(*file).convertedMark.toStdString(), std::string(" -live ")); + file->setValue(importprefs::kConvertedMarkKey, " pedal"); + CHECK_EQ(importprefs::read(*file).convertedMark.toStdString(), std::string(" pedal")); + file->setValue(importprefs::kConvertedMarkKey, "- live -"); // and inside + CHECK_EQ(importprefs::read(*file).convertedMark.toStdString(), std::string("- live -")); + file->setValue(importprefs::kConvertedMarkKey, ""); + file->saveIfNeeded(); + // the rule itself + CHECK_EQ(importprefs::markAsTyped("").toStdString(), std::string("")); + CHECK_EQ(importprefs::markAsTyped(" ").toStdString(), std::string("")); + CHECK_EQ(importprefs::markAsTyped(" x ").toStdString(), std::string(" x ")); + } + + // --- the keys are the ones a settings file shows, beside each other --- + { + auto file = open(work); + CHECK_EQ(std::string(importprefs::kNormalizeOnUploadKey), std::string("normalizeOnUpload")); + CHECK_EQ(std::string(importprefs::kTargetLufsKey), std::string("normalizeTargetLufs")); + CHECK_EQ(std::string(importprefs::kConvertedMarkKey), std::string("convertedUploadMark")); + CHECK(file->containsKey("normalizeOnUpload")); + CHECK(file->containsKey("normalizeTargetLufs")); + CHECK(file->containsKey("convertedUploadMark")); + } + + // --- an absent mark key reads as the default even when the others are set --- + { + { + auto file = open(work); + file->removeValue(importprefs::kConvertedMarkKey); + file->saveIfNeeded(); + } + auto file = open(work); + const ImportPrefs prefs = importprefs::read(*file); + CHECK_EQ(prefs.convertedMark.toStdString(), std::string("-pedal")); + CHECK(prefs.normalizeOnUpload); + } + + // Every PropertiesFile above is closed; a cleanup that fails is a failure. + CHECK(work.deleteRecursively()); + return testkit::summary("import_prefs"); +} diff --git a/tests/mp3_import_tests.cpp b/tests/mp3_import_tests.cpp index 815bc16..45d4cd6 100644 --- a/tests/mp3_import_tests.cpp +++ b/tests/mp3_import_tests.cpp @@ -127,6 +127,16 @@ int main() CHECK(r.wasOk()); if (r.wasOk()) { CHECK(out.converted); // an mp3 is always a conversion, never a pass-through + CHECK(out.rebuilt); // ...and a rebuild: a decoder made these samples + // The fact about the source is its codec, not the decoder's float32 + // (issue #139): the report must not describe an mp3 as "32-bit float". + CHECK(out.sourceFormat.has_value()); + if (out.sourceFormat.has_value()) { + CHECK(!out.sourceFormat->factual); // a decoder's samples, not the file's + CHECK_EQ(out.sourceFormat->encoding.toStdString(), std::string("MP3")); + CHECK_EQ(wavimport::describeConversion(*out.sourceFormat).toStdString(), + std::string("MP3 \xe2\x86\x92 32-bit float")); + } juce::MemoryBlock raw; CHECK(out.file.loadFileAsData(raw)); const auto info = wav::readWavInfo(wav::BytesView( diff --git a/tests/slot_story_tests.cpp b/tests/slot_story_tests.cpp index f99af7c..1fd2bec 100644 --- a/tests/slot_story_tests.cpp +++ b/tests/slot_story_tests.cpp @@ -235,6 +235,7 @@ int main() // a push carries the fact behind a normalize line, or alone, or not at all const auto pushLine = story::tell({ .kind = "push", .beforeBody = longer, .afterBody = shorter, .takeName = "take.wav", .take = story::Take::onCard, + .status = "done", .note = "normalized -3.2 dB; " + std::string(story::kNoteLengthReplaced) }); CHECK(pushLine.detail.find("take.wav") == 0); CHECK(pushLine.detail.find(" - note length replaced by bars") == pushLine.detail.size() - std::string(" - note length replaced by bars").size()); @@ -242,13 +243,74 @@ int main() .note = std::string(story::kNoteLengthReplaced) }); CHECK_EQ(alone.detail, std::string("take.wav - note length replaced by bars")); const auto quiet = story::tell({ .kind = "push", .takeName = "take.wav", .take = story::Take::onCard, - .note = "normalized -3.2 dB" }); - CHECK_EQ(quiet.detail, std::string("take.wav")); + .status = "done", .note = "normalized -3.2 dB" }); + CHECK_EQ(quiet.detail, std::string("take.wav - normalized -3.2 dB")); // the fact belongs to push and trim: a rename's note is a rename's note const auto other = story::tell({ .kind = "rename", .take = story::Take::none, .note = std::string(story::kNoteLengthReplaced) }); CHECK(other.detail.find("note length") == std::string::npos); } + // --- a push row says what the job said about the take (issue #139) --- + // + // The row names the take as it landed — the mark is in the name — keeps + // its numbers, and then draws the note's sentences: the rebuild's facts, + // the normalization's. The note-length fact keeps its own place at the + // end, drawn once, and is left out of the sentences. + { + const auto converted = story::tell({ .kind = "push", .beforeBody = trimmed, .afterBody = pushed, + .takeName = "song-pedal.wav", .take = story::Take::onCard, + .status = "done", + .note = "48000 Hz, 24-bit, mono \xe2\x86\x92 44100 Hz, 32-bit float, stereo" }); + CHECK_EQ(converted.detail, + std::string("song-pedal.wav - 4:36 - 111.1 BPM - 48000 Hz, 24-bit, mono \xe2\x86\x92 44100 Hz, 32-bit float, stereo")); + CHECK_EQ(converted.audio, std::string("in the slot now")); + + const auto both = story::tell({ .kind = "push", .beforeBody = trimmed, .afterBody = pushed, + .takeName = "song-pedal.wav", .take = story::Take::onCard, + .status = "done", + .note = "24-bit \xe2\x86\x92 32-bit float; normalized +3.0 dB; " + + std::string(story::kNoteLengthReplaced) }); + CHECK_EQ(both.detail, + std::string("song-pedal.wav - 4:36 - 111.1 BPM - 24-bit \xe2\x86\x92 32-bit float; normalized +3.0 dB" + " - note length replaced by bars")); + + // no bodies (a row an older store kept): the name and the sentence + const auto bare = story::tell({ .kind = "push", .takeName = "song.wav", .take = story::Take::kept, + .status = "done", .note = "MP3 \xe2\x86\x92 32-bit float" }); + CHECK_EQ(bare.detail, std::string("song.wav - MP3 \xe2\x86\x92 32-bit float")); + + // a note that is only the fact adds nothing twice + const auto onlyFact = story::tell({ .kind = "push", .takeName = "take.wav", .take = story::Take::onCard, + .status = "done", + .note = std::string(story::kNoteLengthReplaced) + "; " }); + CHECK_EQ(onlyFact.detail, std::string("take.wav - note length replaced by bars")); + + // a failed push's note is its error, and the row's state already says + // it failed: the line keeps its old shape, the error is not a sentence + const auto failed = story::tell({ .kind = "push", .takeName = "take.wav", .take = story::Take::onCard, + .status = "failed", + .note = "slot 9 already has audio (loop.wav); pass force to replace" }); + CHECK_EQ(failed.detail, std::string("take.wav")); + const auto failedBodies = story::tell({ .kind = "push", .beforeBody = trimmed, .afterBody = pushed, + .takeName = "take.wav", .take = story::Take::onCard, + .status = "failed", .note = "cannot write MEMORY1.RC0" }); + CHECK_EQ(failedBodies.detail, std::string("take.wav - 4:36 - 111.1 BPM")); + // an interrupted one, and a row an older build wrote with no status: the same + const auto interrupted = story::tell({ .kind = "push", .takeName = "take.wav", .take = story::Take::onCard, + .status = "interrupted", .note = "24-bit \xe2\x86\x92 32-bit float" }); + CHECK_EQ(interrupted.detail, std::string("take.wav")); + const auto unknown = story::tell({ .kind = "push", .takeName = "take.wav", .take = story::Take::onCard, + .note = "24-bit \xe2\x86\x92 32-bit float" }); + CHECK_EQ(unknown.detail, std::string("take.wav")); + + // the helper itself: the fact is dropped wherever it sits, the rest keeps its order + CHECK_EQ(story::sentencesBesides("a; b; c", "b"), std::string("a; c")); + CHECK_EQ(story::sentencesBesides("b; a", "b"), std::string("a")); + CHECK_EQ(story::sentencesBesides("b", "b"), std::string("")); + CHECK_EQ(story::sentencesBesides("", "b"), std::string("")); + CHECK_EQ(story::sentencesBesides("a;b", "b"), std::string("a;b")); // not the join + } + return testkit::summary("slot_story_tests"); } diff --git a/tests/upload_mark_tests.cpp b/tests/upload_mark_tests.cpp new file mode 100644 index 0000000..a7e729c --- /dev/null +++ b/tests/upload_mark_tests.cpp @@ -0,0 +1,162 @@ +// Copyright (c) 2026 Darwin's Cat. Part of Looper Cat — see LICENSE. +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// The name a pushed file lands under (issue #139), from the rule as stated: +// a rebuilt upload is .wav, a repacked one is +// .wav, an untouched one keeps its name, the stem keeps its own +// dots, the suffix may be empty, a suffix a FAT name cannot hold is refused — +// not stripped — and only where it is used, and a name longer than a FAT name +// can be is refused, never trimmed. + +#include "support.hpp" + +#include "../app/UploadMark.h" + +#include +#include + +using namespace loopercat; + +int run() +{ + using uploadmark::Audio; + + // --- the default is the spelling existing cards already carry --- + CHECK_EQ(std::string(uploadmark::kDefaultSuffix), std::string("-pedal")); + + // --- a rebuilt upload: stem + suffix + .wav, the extension changed on purpose --- + CHECK_EQ(uploadmark::landedName("song.mp3", "-pedal", Audio::rebuilt), std::string("song-pedal.wav")); + CHECK_EQ(uploadmark::landedName("take.wav", "-pedal", Audio::rebuilt), std::string("take-pedal.wav")); + CHECK_EQ(uploadmark::landedName("Take.WAV", "-pedal", Audio::rebuilt), std::string("Take-pedal.wav")); + + // --- an empty suffix: the extension still changes, nothing else --- + CHECK_EQ(uploadmark::landedName("song.mp3", "", Audio::rebuilt), std::string("song.wav")); + CHECK_EQ(uploadmark::landedName("loop.aiff", "", Audio::rebuilt), std::string("loop.wav")); + + // --- a repacked upload: its own stem, the honest extension, no mark whatever the suffix --- + CHECK_EQ(uploadmark::landedName("song.mp3", "-pedal", Audio::repackaged), std::string("song.wav")); + CHECK_EQ(uploadmark::landedName("Take.WAV", "-pedal", Audio::repackaged), std::string("Take.wav")); + CHECK_EQ(uploadmark::landedName("v1.2 mix.aif", "-pedal", Audio::repackaged), + std::string("v1.2 mix.wav")); + CHECK_EQ(uploadmark::landedName("export.wav", "", Audio::repackaged), std::string("export.wav")); + + // --- an untouched upload keeps its name whatever the suffix says --- + CHECK_EQ(uploadmark::landedName("take.wav", "-pedal", Audio::untouched), std::string("take.wav")); + CHECK_EQ(uploadmark::landedName("take.wav", "", Audio::untouched), std::string("take.wav")); + CHECK_EQ(uploadmark::landedName("Take.WAV", "-pedal", Audio::untouched), std::string("Take.WAV")); + + // --- the stem keeps its own dots; only the last one is the extension --- + CHECK_EQ(uploadmark::landedName("v1.2 mix.aif", "-pedal", Audio::rebuilt), + std::string("v1.2 mix-pedal.wav")); + CHECK_EQ(uploadmark::landedName("set.final.v3.flac", " live", Audio::rebuilt), + std::string("set.final.v3 live.wav")); + + // --- no extension at all: the whole name is the stem --- + CHECK_EQ(uploadmark::landedName("song", "-pedal", Audio::rebuilt), std::string("song-pedal.wav")); + // a leading dot is not an extension either + CHECK_EQ(uploadmark::landedName(".hidden", "-pedal", Audio::rebuilt), std::string(".hidden-pedal.wav")); + // a trailing dot is an empty extension: the stem ends before it + CHECK_EQ(uploadmark::landedName("song.", "-pedal", Audio::rebuilt), std::string("song-pedal.wav")); + + // --- a UTF-8 name survives byte for byte --- + CHECK_EQ(uploadmark::landedName("caf\xc3\xa9 take.mp3", "\xe2\x80\x93pedal", Audio::rebuilt), + std::string("caf\xc3\xa9 take\xe2\x80\x93pedal.wav")); + + // --- a suffix a FAT name cannot hold is refused, with the character named --- + for (const char c : std::string("/\\:*?\"<>|")) { + const std::string bad = std::string("-ped") + c + "al"; + CHECK_THROWS(uploadmark::assertSuffix(bad), std::string(1, c)); + CHECK_THROWS(uploadmark::landedName("song.mp3", bad, Audio::rebuilt), std::string(1, c)); + // ...only where the mark is used: a bad mark hand-edited into the + // settings file must not stop an upload that never wears it + CHECK_EQ(uploadmark::landedName("take.wav", bad, Audio::untouched), std::string("take.wav")); + CHECK_EQ(uploadmark::landedName("take.wav", bad, Audio::repackaged), std::string("take.wav")); + } + CHECK_THROWS(uploadmark::assertSuffix(std::string("-pedal\t")), "control"); + CHECK_THROWS(uploadmark::assertSuffix(std::string("-pedal\n")), "control"); + CHECK_THROWS(uploadmark::assertSuffix(std::string("\x01", 1)), "control"); + + // --- what a FAT name can hold is accepted as typed: spaces, dots, unicode --- + uploadmark::assertSuffix(""); + uploadmark::assertSuffix(" (pedal)"); + uploadmark::assertSuffix("-v1.2"); + uploadmark::assertSuffix("\xe2\x80\x93pedal"); + uploadmark::assertSuffix("_PEDAL_"); + ++testkit::checksRun; // the five lines above threw nothing + + // --- the card's limit: 238 UTF-16 units for the whole name, counted, never trimmed --- + // + // FAT's own 255 is not the binding limit: the card is read on Windows, + // whose MAX_PATH (260, terminator included) leaves 238 under the + // 21-character card folder (fatname::kMaxLandedUnits). The review's + // reproductions — a 247-character stem with "-pedal", a 252-character + // stem, a 260-character mark, and the 239-unit name that passed the + // first cap — are all refused here, before anything reaches the card. + CHECK_EQ(uploadmark::kMaxSuffixUnits, std::size_t { 233 }); + { + const std::string stem228(228, 's'); + CHECK_EQ(uploadmark::landedName(stem228 + ".mp3", "-pedal", Audio::rebuilt).size(), + std::size_t { 238 }); // exactly the limit: allowed + const std::string stem229(229, 's'); + CHECK_THROWS(uploadmark::landedName(stem229 + ".mp3", "-pedal", Audio::rebuilt), "at most 238"); + const std::string stem230(230, 's'); + CHECK_THROWS(uploadmark::landedName(stem230 + ".mp3", "-pedal", Audio::rebuilt), "or the mark"); + // the same stem with no mark fits: it was the mark that pushed it over + CHECK_EQ(uploadmark::landedName(stem230 + ".mp3", "", Audio::rebuilt).size(), std::size_t { 234 }); + const std::string stem247(247, 's'); + CHECK_THROWS(uploadmark::landedName(stem247 + ".mp3", "-pedal", Audio::rebuilt), "at most 238"); + // repacked names are measured too, and the advice names the file only + const std::string stem235(235, 's'); + CHECK_THROWS(uploadmark::landedName(stem235 + ".mp3", "", Audio::repackaged), "shorten the file name"); + const std::string stem252(252, 's'); + CHECK_THROWS(uploadmark::landedName(stem252 + ".mp3", "", Audio::repackaged), "at most 238"); + // an untouched name is the host's own and is not measured here (the core does) + CHECK_EQ(uploadmark::landedName(std::string(300, 'u') + ".wav", "-pedal", Audio::untouched).size(), + std::size_t { 304 }); + } + { + const std::string mark260(260, 'm'); + CHECK_THROWS(uploadmark::assertSuffix(mark260), "at most 233"); + CHECK_THROWS(uploadmark::landedName("song.mp3", mark260, Audio::rebuilt), "at most 233"); + const std::string mark234(234, 'm'); + CHECK_THROWS(uploadmark::assertSuffix(mark234), "at most 233"); + const std::string mark233(233, 'm'); + uploadmark::assertSuffix(mark233); + ++testkit::checksRun; // the longest mark a name can carry + // ...and with the shortest stem it is exactly the limit + CHECK_EQ(uploadmark::landedName("s.mp3", mark233, Audio::rebuilt).size(), std::size_t { 238 }); + } + { + // units, not bytes: 100 "é" are 200 bytes and 100 units + std::string accented; + for (int i = 0; i < 100; ++i) + accented += "\xc3\xa9"; + const std::string landed = uploadmark::landedName(accented + ".mp3", "-pedal", Audio::rebuilt); + CHECK_EQ(landed.size(), std::size_t { 210 }); + // the limit is in units: a name of 238 units may be 239 bytes... + const std::string stem233(233, 's'); + CHECK_EQ(uploadmark::landedName(stem233 + ".mp3", "\xc3\xa9", Audio::rebuilt).size(), + std::size_t { 239 }); // 233 + 1 unit + 4 = 238 units in 239 bytes: allowed + // ...or 240 bytes, when the extra character is a surrogate pair (2 units) + const std::string stem232(232, 's'); + CHECK_EQ(uploadmark::landedName(stem232 + ".mp3", "\xf0\x9f\x98\x80", Audio::rebuilt).size(), + std::size_t { 240 }); // 232 + 2 units (4 bytes) + 4 = 238 units in 240 bytes: allowed + // and the pair on a stem one longer is the 239th unit + CHECK_THROWS(uploadmark::landedName(stem233 + ".mp3", "\xf0\x9f\x98\x80", Audio::rebuilt), + "at most 238"); + } + + return testkit::summary("upload_mark"); +} + +// A refusal the suite did not expect must name itself, not abort the process +// with no line (the Windows lane reports an abort as a bare exit code). +int main() +{ + try { + return run(); + } catch (const std::exception& e) { + testkit::fail(std::string("escaped: ") + e.what(), __FILE__, __LINE__); + return testkit::summary("upload_mark"); + } +} diff --git a/tests/wav_import_tests.cpp b/tests/wav_import_tests.cpp index 358b347..4aa74b4 100644 --- a/tests/wav_import_tests.cpp +++ b/tests/wav_import_tests.cpp @@ -6,6 +6,11 @@ // else JUCE reads becomes 44.1 kHz stereo float32; what cannot become that // fails plainly. The field case that started this: DAW 24-bit exports wear // WAVE_FORMAT_EXTENSIBLE headers the strict core parser refuses. +// +// Issue #139 adds what a conversion reports about itself — the source's +// rate, sample format and channels, said only where they differ from the +// pedal's — and where it lives: a directory per job under import-tmp, gone +// with the job whichever way the job ends. #include "support.hpp" @@ -13,13 +18,20 @@ #include +#include #include #include #include #include +#include #include #include +#include + +#if JUCE_MAC +#include +#endif using namespace loopercat; @@ -71,6 +83,94 @@ std::vector extensiblePcm24(int frames) return b; } +// A WAVE_FORMAT_EXTENSIBLE 32-bit float 44.1 kHz stereo file — the shape a +// DAW's float export wears. The pedal's strict gate refuses the header +// (Info::format() reads tag 0xFFFE as pcm32), so it goes through the +// converter; but every sample already is what the pedal wants. +std::vector extensibleFloat32(int frames) +{ + std::vector b; + const auto ascii = [&b](std::string_view s) { + for (const char c : s) + b.push_back(static_cast(c)); + }; + const auto p16 = [&b](int v) { + b.push_back(static_cast(v & 0xff)); + b.push_back(static_cast((v >> 8) & 0xff)); + }; + const auto p32 = [&p16](long long v) { + p16(static_cast(v & 0xffff)); + p16(static_cast((v >> 16) & 0xffff)); + }; + const auto pf = [&p32](float v) { + p32(static_cast(std::bit_cast(v))); + }; + const int blockAlign = 2 * 4; + const int dataSize = frames * blockAlign; + ascii("RIFF"); p32(4 + 8 + 40 + 8 + dataSize); ascii("WAVE"); + ascii("fmt "); p32(40); + p16(0xfffe); // WAVE_FORMAT_EXTENSIBLE + p16(2); p32(44100); p32(44100 * blockAlign); p16(blockAlign); p16(32); + p16(22); // cbSize + p16(32); // valid bits + p32(3); // channel mask: L | R + // SubFormat GUID: KSDATAFORMAT_SUBTYPE_IEEE_FLOAT + p32(3); p16(0); p16(0x10); + for (const int c : { 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }) + b.push_back(static_cast(c)); + ascii("data"); p32(dataSize); + // A signal with texture, not a bare tone — a slow ramp rides on the sine + // so neighbouring frames differ and a sample that moved would be seen — + // kept within full scale, so the loudness meter reads it as audio. + const double w = 2.0 * std::numbers::pi * 997.0 / 44100.0; + for (int i = 0; i < frames; ++i) { + pf(static_cast(0.4 * std::sin(w * i) + 1.0e-5 * (i % 1000))); + pf(static_cast(-0.3 * std::cos(w * i * 1.3))); + } + return b; +} + +// Every sample of a file, through JUCE's reader (which reads the +// extensible header the core refuses), channels side by side. +std::vector samplesOf(const juce::File& f, int channels, int frames) +{ + juce::AudioFormatManager formats; + formats.registerBasicFormats(); + std::unique_ptr reader(formats.createReaderFor(f)); + std::vector out; + if (reader == nullptr) + return out; + juce::AudioBuffer buf(channels, frames); + reader->read(&buf, 0, frames, 0, true, true); + for (int c = 0; c < channels; ++c) + for (int i = 0; i < frames; ++i) + out.push_back(buf.getSample(c, i)); + return out; +} + +// A file written by one of JUCE's own encoders, from a mono frame-index ramp +// at the given rate and width — the FLAC and Ogg sources a player may drop. +juce::File encodeRamp(juce::AudioFormat& format, const juce::File& dest, double sampleRate, + int bits, int frames) +{ + std::unique_ptr stream = dest.createOutputStream(); + if (stream == nullptr) + return {}; + std::unique_ptr writer = format.createWriterFor( + stream, juce::AudioFormatWriterOptions {} + .withSampleRate(sampleRate) + .withNumChannels(1) + .withBitsPerSample(bits)); + if (writer == nullptr) + return {}; + juce::AudioBuffer buf(1, frames); + for (int i = 0; i < frames; ++i) + buf.setSample(0, i, static_cast(i - frames / 2) / 32768.0f); + writer->writeFromAudioSampleBuffer(buf, 0, frames); + writer.reset(); + return dest; +} + // Mono pcm16 with a frame-index ramp — testkit's rampFill is stereo-only, // and the mono test needs a signal, not silence, to prove the duplication. std::vector monoRamp(int frames) @@ -98,6 +198,15 @@ std::vector monoRamp(int frames) return b; } +// A file's bytes as the core reads them — kept alive by the caller's block. +std::vector fileBytes(const juce::File& f) +{ + juce::MemoryBlock raw; + testkit::check(f.loadFileAsData(raw), "loadFileAsData", __FILE__, __LINE__); + const auto* data = static_cast(raw.getData()); + return std::vector(data, data + raw.getSize()); +} + wav::Info infoOf(const juce::File& f) { juce::MemoryBlock raw; @@ -209,6 +318,7 @@ int main() wavimport::Prepared p; CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); CHECK(!p.converted); + CHECK(!p.rebuilt); CHECK(p.file == src); } @@ -253,6 +363,7 @@ int main() wavimport::Prepared p; CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); CHECK(p.converted); + CHECK(p.rebuilt); // resampled: these are not the source's samples const wav::Info out = infoOf(p.file); CHECK_EQ(out.sampleRate, wav::kSampleRate); CHECK_EQ(out.format(), std::string("float32")); @@ -317,6 +428,7 @@ int main() wavimport::Prepared p; CHECK(wavimport::prepare(src, tmp, p, { .normalizeTargetLufs = -18.0 }).wasOk()); CHECK(p.converted); + CHECK(p.rebuilt); // a gain went in: the samples are not the source's CHECK(p.file != src); CHECK(p.normalize.has_value()); CHECK(p.normalize->measurable); @@ -429,6 +541,447 @@ int main() CHECK(!measureLufs(p.file).has_value()); // still silence on the way out } - work.deleteRecursively(); + // --- what the rebuild changed, as facts (issue #139) --- + + // The arrow the report draws — the one describeNormalize already uses, + // so a note carrying both sentences reads with one. + const std::string arrow = " \xe2\x86\x92 "; + + // 48 kHz, 24-bit, mono: every fact differs from the pedal's, and the + // sentence says all three — source on the left, the pedal on the right. + { + const auto bytes = testkit::syntheticWav( + { .tag = 1, .channels = 1, .sampleRate = 48000, .bits = 24, .frames = 4800 }); + const juce::File src = writeTemp(work, "daw-mono-48k.wav", bytes); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.converted); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK_EQ(p.sourceFormat->sampleRate, 48000); + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("24-bit")); + CHECK_EQ(p.sourceFormat->channels, 1); + CHECK_EQ(wavimport::describeConversion(*p.sourceFormat).toStdString(), + "48000 Hz, 24-bit, mono" + arrow + "44100 Hz, 32-bit float, stereo"); + } + } + + // A pedal-ready file passes through with no facts: there was no rebuild + // to report, and no directory to own. + { + const auto bytes = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 4410 }); + const juce::File src = writeTemp(work, "ready-facts.wav", bytes); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(!p.converted); + CHECK(!p.sourceFormat.has_value()); + CHECK(p.jobDir.path() == juce::File()); + } + + // A pedal-ready file rewritten for its loudness alone IS a conversion, + // and its facts match the pedal on every point — so the sentence is + // empty: "32-bit float → 32-bit float" is nothing anyone needs told. + { + const juce::File src = + writeTemp(work, "quiet-facts.wav", sineWav(3 * 44100, 2, dbAmp(-28.0))); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, { .normalizeTargetLufs = -18.0 }).wasOk()); + CHECK(p.converted); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK_EQ(p.sourceFormat->sampleRate, 44100); + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("32-bit float")); + CHECK_EQ(p.sourceFormat->channels, 2); + CHECK(wavimport::describeConversion(*p.sourceFormat).isEmpty()); + } + CHECK(p.normalize.has_value()); + CHECK(p.normalize->measurable); // the gain is the only story here + } + + // The field case keeps its bit depth as the fact, not the header's shape. + { + const juce::File src = writeTemp(work, "daw-export-facts.wav", extensiblePcm24(4410)); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) + CHECK_EQ(wavimport::describeConversion(*p.sourceFormat).toStdString(), + "24-bit" + arrow + "32-bit float"); + } + + // --- repacked, not rebuilt: a float export wearing an extensible header --- + // + // The pedal's gate refuses the header, so the file goes through the + // converter — but its rate, format and channels are already the pedal's + // and no gain went in. Converted (it lives in a job directory), not + // rebuilt (the mark must not claim the samples changed), no sentence, + // and every sample out equals every sample in. + { + const int frames = 4410; + const juce::File src = writeTemp(work, "daw-float.wav", extensibleFloat32(frames)); + CHECK_THROWS(wav::assertUploadable(infoOf(src)), "format"); // the gate's view of it + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.converted); + CHECK(!p.rebuilt); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK_EQ(p.sourceFormat->sampleRate, 44100); + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("32-bit float")); + CHECK_EQ(p.sourceFormat->channels, 2); + CHECK(p.sourceFormat->factual); // JUCE's WAV reader: the width is the file's + CHECK(!wavimport::differsFromTarget(*p.sourceFormat)); + CHECK(wavimport::describeConversion(*p.sourceFormat).isEmpty()); + } + const std::vector in = samplesOf(src, 2, frames); + const std::vector out = samplesOf(p.file, 2, frames); + CHECK_EQ(in.size(), std::size_t(2 * frames)); + CHECK(in == out); // bit for bit, not within a tolerance + CHECK_EQ(infoOf(p.file).format(), std::string("float32")); // canonical now + } + + // The same file, normalize ON and off target: now the samples did change. + { + const juce::File src = writeTemp(work, "daw-float-quiet.wav", extensibleFloat32(3 * 44100)); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, { .normalizeTargetLufs = -18.0 }).wasOk()); + CHECK(p.converted); + CHECK(p.rebuilt); + CHECK(p.normalize.has_value() && p.normalize->measurable && !p.normalize->untouched); + } + + // --- the encoding fact per format: FLAC keeps its width, Ogg names its codec --- + { + juce::FlacAudioFormat flac; + const juce::File src = encodeRamp(flac, work.getChildFile("ramp.flac"), 48000.0, 24, 4800); + CHECK(src.existsAsFile()); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.converted && p.rebuilt); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK(p.sourceFormat->factual); // FLAC keeps the width, and its reader reports it + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("24-bit")); + CHECK_EQ(wavimport::describeConversion(*p.sourceFormat).toStdString(), + "48000 Hz, 24-bit, mono" + arrow + "44100 Hz, 32-bit float, stereo"); + } + } + { + juce::OggVorbisAudioFormat ogg; + const juce::File src = encodeRamp(ogg, work.getChildFile("ramp.ogg"), 44100.0, 16, 4410); + CHECK(src.existsAsFile()); + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.converted && p.rebuilt); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK(!p.sourceFormat->factual); // a decoder's samples + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("Ogg Vorbis")); + CHECK_EQ(wavimport::describeConversion(*p.sourceFormat).toStdString(), + "Ogg Vorbis, mono" + arrow + "32-bit float, stereo"); + } + } + + // An Ogg stream inside a WAV container (fmt tag 0x6771): WavAudioFormat + // hands it to the Ogg reader, so the codec must be read off the READER's + // name — judged by the format object it would read "16-bit float". + { + juce::OggVorbisAudioFormat ogg; + const juce::File plain = encodeRamp(ogg, work.getChildFile("inner.ogg"), 44100.0, 16, 4410); + juce::MemoryBlock oggBytes; + CHECK(plain.loadFileAsData(oggBytes)); + std::vector b; + const auto ascii = [&b](std::string_view t) { + for (const char c : t) + b.push_back(static_cast(c)); + }; + const auto p16 = [&b](int v) { + b.push_back(static_cast(v & 0xff)); + b.push_back(static_cast((v >> 8) & 0xff)); + }; + const auto p32 = [&p16](long long v) { + p16(static_cast(v & 0xffff)); + p16(static_cast((v >> 16) & 0xffff)); + }; + const auto payload = static_cast(oggBytes.getSize()); + const long long padded = payload + (payload % 2); + ascii("RIFF"); p32(4 + 8 + 16 + 8 + padded); ascii("WAVE"); + ascii("fmt "); p32(16); + p16(0x6771); p16(1); p32(44100); p32(0); p16(0); p16(16); // WAVE_FORMAT_OGG_VORBIS_MODE_3_PLUS + ascii("data"); p32(payload); + const auto* bytes = static_cast(oggBytes.getData()); + b.insert(b.end(), bytes, bytes + oggBytes.getSize()); + if (padded != payload) + b.push_back(0); + const juce::File src = writeTemp(work, "ogg-in-wav.wav", b); + wavimport::Prepared p; + const juce::Result r = wavimport::prepare(src, tmp, p, {}); + CHECK(r.wasOk()); + if (r.wasOk()) { + CHECK(p.converted && p.rebuilt); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK(!p.sourceFormat->factual); + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("Ogg Vorbis")); + CHECK_EQ(wavimport::describeConversion(*p.sourceFormat).toStdString(), + "Ogg Vorbis, mono" + arrow + "32-bit float, stereo"); + } + } + } + + // --- what the system decodes is rebuilt, whatever its facts read --- + // + // A float64 WAV: JUCE's own reader refuses it (over 32 bits), and on + // macOS CoreAudio takes it and reports 32-bit float, 44.1 kHz, stereo — + // "nothing differs" by the facts, while every sample was re-quantised. + // Positive evidence only: a source that no sample reader vouched for is + // rebuilt, and the sentence says what it can. + { + const auto bytes = testkit::syntheticWav({ .tag = 3, .bits = 64, .frames = 4410 }); + const juce::File src = writeTemp(work, "f64.wav", bytes); + CHECK_THROWS(wav::assertUploadable(infoOf(src)), "format"); // the gate: float64 + wavimport::Prepared p; + const juce::Result r = wavimport::prepare(src, tmp, p, {}); +#if JUCE_MAC + CHECK(r.wasOk()); // CoreAudio is registered: dropping it would lose AAC/M4A import +#endif + if (r.wasOk()) { + CHECK(p.converted); + CHECK(p.rebuilt); + CHECK(p.sourceFormat.has_value()); + if (p.sourceFormat.has_value()) { + CHECK(!p.sourceFormat->factual); + CHECK_EQ(p.sourceFormat->encoding.toStdString(), std::string("decoded by the system")); + CHECK_EQ(wavimport::describeConversion(*p.sourceFormat).toStdString(), + "decoded by the system" + arrow + "32-bit float"); + } + } else { + // no system decoder for it on this platform: refused, never guessed at + CHECK(r.getErrorMessage().contains("not an audio file")); + } + } + + // --- a WAV cut short is refused, never padded --- + // + // The header claims more frames than the file holds. The core gate calls + // it truncated; JUCE's reader would pad the missing frames with silence + // and the pedal would get a loop twice as long as its audio, under the + // player's own name. + { + const auto cutFloat = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 4410, .truncateBy = 4410 * 4 }); + const juce::File src = writeTemp(work, "cut-float.wav", cutFloat); + wavimport::Prepared p; + const juce::Result r = wavimport::prepare(src, tmp, p, {}); + CHECK(r.failed()); + CHECK_EQ(r.getErrorMessage().toStdString(), + std::string("cut-float.wav is cut short: its data chunk claims 17640 bytes more than the file holds")); + CHECK(!p.converted); + CHECK(p.file == juce::File()); + // the same with pcm16 — the shape JUCE would otherwise convert and pad + const auto cutPcm = testkit::syntheticWav({ .frames = 4410, .truncateBy = 4410 * 2 }); + const juce::File src16 = writeTemp(work, "cut-pcm.wav", cutPcm); + wavimport::Prepared q; + const juce::Result r16 = wavimport::prepare(src16, tmp, q, {}); + CHECK(r16.failed()); + CHECK(r16.getErrorMessage().contains("cut short")); + // normalize ON changes nothing about the refusal + wavimport::Prepared n; + CHECK(wavimport::prepare(src16, tmp, n, { .normalizeTargetLufs = -18.0 }).failed()); + // and one byte short is short + const auto cutOne = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 4410, .truncateBy = 1 }); + wavimport::Prepared o; + CHECK(wavimport::prepare(writeTemp(work, "cut-one.wav", cutOne), tmp, o, {}).failed()); + } + + // --- a header a writer never finalised is refused by that name --- + { + auto bytes = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 4410 }); + for (int i = 40; i < 44; ++i) + bytes[static_cast(i)] = 0xff; // the data chunk's size field + wavimport::Prepared p; + const juce::Result r = wavimport::prepare(writeTemp(work, "unfinalised.wav", bytes), tmp, p, {}); + CHECK(r.failed()); + CHECK_EQ(r.getErrorMessage().toStdString(), + std::string("unfinalised.wav was never finalised: its data chunk's size is still unset")); + CHECK(!p.converted); + } + + // --- a metadata trailer cut off after the audio: imported, every sample intact --- + // + // The core calls the file truncated and the gate refuses it, so it goes + // through the converter — as a repack: a pedal-ready file whose only + // flaw is a LIST chunk that ends early loses nothing. + { + const int frames = 4410; + auto bytes = sineWav(frames, 2, dbAmp(-12.0)); + for (const char c : std::string("LIST")) bytes.push_back(static_cast(c)); + for (const int c : { 26, 0, 0, 0 }) bytes.push_back(static_cast(c)); + for (int i = 0; i < 10; ++i) bytes.push_back(0); + const juce::File src = writeTemp(work, "cut-trailer.wav", bytes); + { + const auto raw = fileBytes(src); + CHECK_THROWS(wav::readWavInfo(wav::BytesView(raw.data(), raw.size())), "truncated"); // the core's view + } + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.converted); + CHECK(!p.rebuilt); + const std::vector in = samplesOf(src, 2, frames); + const std::vector out = samplesOf(p.file, 2, frames); + CHECK_EQ(in.size(), std::size_t(2 * frames)); + CHECK(in == out); + CHECK_EQ(infoOf(p.file).frames, std::int64_t(frames)); + } + + // --- two data chunks: refused, nothing converted --- + // + // The core refuses the file as malformed; a decoder would keep the last + // chunk and drop the first without a word (the review saw 88200 frames + // vanish and the rest land unmarked under the player's own name). + { + auto bytes = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 88200 }); + const auto second = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 44100 }); + bytes.insert(bytes.end(), second.begin() + 36, second.end()); // its data chunk only + wavimport::Prepared p; + const juce::Result r = wavimport::prepare(writeTemp(work, "two-data.wav", bytes), tmp, p, {}); + CHECK(r.failed()); + CHECK_EQ(r.getErrorMessage().toStdString(), std::string("two-data.wav has two data chunks")); + CHECK(!p.converted); + CHECK(p.file == juce::File()); + CHECK_EQ(tmp.getNumberOfChildFiles(juce::File::findFilesAndDirectories), 0); + } + + // --- the sentence says only what changed --- + { + using wavimport::SourceFormat; + const auto say = [](const SourceFormat& f) { + return wavimport::describeConversion(f).toStdString(); + }; + CHECK_EQ(say({ 48000, "24-bit", 1 }), + "48000 Hz, 24-bit, mono" + arrow + "44100 Hz, 32-bit float, stereo"); + CHECK_EQ(say({ 44100, "24-bit", 2 }), "24-bit" + arrow + "32-bit float"); + CHECK_EQ(say({ 44100, "32-bit float", 2 }), std::string("")); + CHECK_EQ(say({ 44100, "16-bit", 2 }), "16-bit" + arrow + "32-bit float"); + CHECK_EQ(say({ 44100, "MP3", 2 }), "MP3" + arrow + "32-bit float"); + CHECK_EQ(say({ 48000, "32-bit float", 2 }), "48000 Hz" + arrow + "44100 Hz"); + CHECK_EQ(say({ 44100, "32-bit float", 1 }), "mono" + arrow + "stereo"); + CHECK_EQ(say({ 96000, "16-bit", 1 }), + "96000 Hz, 16-bit, mono" + arrow + "44100 Hz, 32-bit float, stereo"); + CHECK_EQ(say({ 22050, "8-bit", 2 }), "22050 Hz, 8-bit" + arrow + "44100 Hz, 32-bit float"); + } + + // --- the conversion's directory: one per job, gone with the job --- + + // A conversion lives in a directory of its own directly under import-tmp, + // under a working name that is nobody's product: the name on the card is + // decided at the push, not here. + { + const auto bytes = testkit::syntheticWav({ .sampleRate = 48000, .frames = 48000 }); + const juce::File src = writeTemp(work, "song.wav", bytes); + juce::File dir; + { + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + CHECK(p.converted); + dir = p.jobDir.path(); + CHECK(dir.isDirectory()); + CHECK(p.file.getParentDirectory() == dir); + CHECK(dir.getParentDirectory() == tmp); + CHECK(!p.file.getFileName().contains("-pedal")); + CHECK(!p.file.getFileName().startsWith("song")); + } + // the push succeeded and the job returned: the directory is gone + CHECK(!dir.exists()); + } + + // The push threw: the directory is gone just the same — the job's scope + // unwinds through the Prepared, whichever way it ends. + { + const auto bytes = testkit::syntheticWav({ .sampleRate = 48000, .frames = 48000 }); + const juce::File src = writeTemp(work, "song-throws.wav", bytes); + juce::File dir; + try { + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + dir = p.jobDir.path(); + CHECK(dir.isDirectory()); + throw Error("slot 9 already has audio"); + } catch (const Error&) { + } + CHECK(!dir.exists()); + } + + // Two files with the same stem in one queue: each conversion in its own + // directory, neither renamed around the other, both readable at once. + { + const juce::File dirA = work.getChildFile("a"); + const juce::File dirB = work.getChildFile("b"); + dirA.createDirectory(); + dirB.createDirectory(); + const juce::File srcA = writeTemp(dirA, "song.wav", + testkit::syntheticWav({ .sampleRate = 48000, .frames = 48000 })); + const juce::File srcB = writeTemp(dirB, "song.wav", monoRamp(1000)); + { + wavimport::Prepared a, b; + CHECK(wavimport::prepare(srcA, tmp, a, {}).wasOk()); + CHECK(wavimport::prepare(srcB, tmp, b, {}).wasOk()); + CHECK(a.converted && b.converted); + CHECK(a.file != b.file); + CHECK(a.jobDir.path() != b.jobDir.path()); + CHECK(a.file.existsAsFile() && b.file.existsAsFile()); + CHECK(std::llabs(infoOf(a.file).frames - 44100) <= 1); // 48000 frames at 48 kHz + CHECK_EQ(infoOf(b.file).frames, 1000); // the mono ramp, untouched + } + // both jobs done: import-tmp holds nothing of either + CHECK_EQ(tmp.getNumberOfChildFiles(juce::File::findFilesAndDirectories), 0); + } + + // The directory follows the Prepared that owns it through a move, and + // dies with the last owner, not the first. + { + const auto bytes = testkit::syntheticWav({ .sampleRate = 48000, .frames = 4800 }); + const juce::File src = writeTemp(work, "song-moved.wav", bytes); + juce::File dir; + { + wavimport::Prepared moved; + { + wavimport::Prepared p; + CHECK(wavimport::prepare(src, tmp, p, {}).wasOk()); + dir = p.jobDir.path(); + moved = std::move(p); + } + CHECK(dir.isDirectory()); // the first owner is gone, the directory is not + CHECK(moved.file.existsAsFile()); + } + CHECK(!dir.exists()); + } + +#if JUCE_MAC + // A delete that fails leaves a line in the operations log beside + // import-tmp's data home — the only trace a leftover has. Denied by + // taking the write bit off import-tmp; root is never denied, so the + // case is skipped for it. + if (geteuid() != 0) { + const juce::File home = work.getChildFile("home"); + const juce::File importTmp = home.getChildFile("import-tmp"); + importTmp.createDirectory(); + const auto bytes = testkit::syntheticWav({ .sampleRate = 48000, .frames = 4800 }); + const juce::File src = writeTemp(work, "song-stuck.wav", bytes); + juce::File dir; + { + wavimport::Prepared p; + CHECK(wavimport::prepare(src, importTmp, p, {}).wasOk()); + dir = p.jobDir.path(); + CHECK(importTmp.setReadOnly(true, false)); + } + CHECK(importTmp.setReadOnly(false, false)); + CHECK(dir.isDirectory()); // the delete was denied... + const juce::String log = home.getChildFile("operations.log").loadFileAsString(); + CHECK(log.contains("import-tmp: could not remove " + dir.getFullPathName())); + } +#endif + + // Every Prepared above is gone; a cleanup that fails is a failure. + CHECK(work.deleteRecursively()); return testkit::summary("wav_import"); } diff --git a/tests/wav_tests.cpp b/tests/wav_tests.cpp index 27cd8c6..4383b3e 100644 --- a/tests/wav_tests.cpp +++ b/tests/wav_tests.cpp @@ -324,5 +324,72 @@ int main() CHECK_THROWS(wav::trackFileName("x.wav", 3, 2), "track out of range"); CHECK_THROWS(wav::trackFileName("x.wav", 2, 1), "track out of range"); + // --- a fault in a RIFF's structure is named, and only a fault (issue #139, review) --- + { + const auto view = [](const std::vector& b) { return wav::BytesView(b.data(), b.size()); }; + const auto whole = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 1000 }); + CHECK(!wav::riffFault(view(whole)).has_value()); + const auto withList = testkit::syntheticWav({ .frames = 1000, .extraChunk = true }); + CHECK(!wav::riffFault(view(withList)).has_value()); + // the fmt chunk's fields are shape, not structure: an extensible tag is no fault + auto extensible = testkit::syntheticWav({ .tag = 0xfffe, .bits = 24, .frames = 10 }); + CHECK(!wav::riffFault(view(extensible)).has_value()); + CHECK_THROWS(wav::readWavInfo(view(extensible)), "format tag"); + + // a data chunk short of its claim: cut, by how much + const auto cut = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 1000, .truncateBy = 1 }); + CHECK_EQ(wav::riffFault(view(cut)).value_or(""), + std::string("is cut short: its data chunk claims 1 bytes more than the file holds")); + const auto cutDeep = testkit::syntheticWav({ .frames = 1000, .truncateBy = 3000 }); + CHECK_EQ(wav::riffFault(view(cutDeep)).value_or(""), + std::string("is cut short: its data chunk claims 3000 bytes more than the file holds")); + CHECK_THROWS(wav::readWavInfo(view(cutDeep)), "truncated"); + + // a data size a writer never set + auto unfinalised = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 1000 }); + for (int i = 40; i < 44; ++i) // RIFF(12) + fmt(8 + 16): the data chunk's size field + unfinalised[static_cast(i)] = 0xff; + CHECK_EQ(wav::riffFault(view(unfinalised)).value_or(""), + std::string("was never finalised: its data chunk's size is still unset")); + + // a metadata trailer cut off after a complete data chunk: no sample is + // missing, so no fault — though readWavInfo still calls it truncated + auto cutTrailer = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 1000 }); + for (const char c : std::string("LIST")) cutTrailer.push_back(static_cast(c)); + for (const int c : { 26, 0, 0, 0 }) cutTrailer.push_back(static_cast(c)); + for (int i = 0; i < 10; ++i) cutTrailer.push_back(0); + CHECK(!wav::riffFault(view(cutTrailer)).has_value()); + CHECK_THROWS(wav::readWavInfo(view(cutTrailer)), "truncated"); + + // two data chunks: a reader would keep one and drop the other + auto twoData = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 10 }); + { + const auto second = testkit::syntheticWav({ .tag = 3, .bits = 32, .frames = 5 }); + twoData.insert(twoData.end(), second.begin() + 36, second.end()); // its data chunk only + } + CHECK_EQ(wav::riffFault(view(twoData)).value_or(""), std::string("has two data chunks")); + CHECK_THROWS(wav::readWavInfo(view(twoData)), "more than one data chunk"); + + // two fmt chunks, a short fmt chunk, no data chunk, no fmt chunk + auto twoFmt = testkit::syntheticWav({ .frames = 10 }); + twoFmt.insert(twoFmt.begin() + 36, twoFmt.begin() + 12, twoFmt.begin() + 36); + CHECK_EQ(wav::riffFault(view(twoFmt)).value_or(""), std::string("has two fmt chunks")); + auto shortFmt = testkit::syntheticWav({ .frames = 10 }); + shortFmt[16] = 8; // the fmt chunk claims 8 bytes + CHECK_EQ(wav::riffFault(view(shortFmt)).value_or(""), + std::string("has a malformed fmt chunk of 8 bytes")); + const auto noData = std::vector(whole.begin(), whole.begin() + 36); + CHECK_EQ(wav::riffFault(view(noData)).value_or(""), std::string("has no data chunk")); + std::vector noFmt(whole.begin(), whole.begin() + 12); + noFmt.insert(noFmt.end(), whole.begin() + 36, whole.end()); + CHECK_EQ(wav::riffFault(view(noFmt)).value_or(""), std::string("has no fmt chunk")); + + // not RIFF/WAVE at all is nobody's fault here + const std::vector text { 'h', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!', '!' }; + CHECK(!wav::riffFault(view(text)).has_value()); + const std::vector tiny { 'R', 'I', 'F', 'F' }; + CHECK(!wav::riffFault(view(tiny)).has_value()); + } + return testkit::summary("wav"); }