Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 64 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,55 @@ phase plan these entries follow.

## [Unreleased]

### Changed

- **The three advertisement switches are one dropdown, because between them they only ever had
three answers.** "Mute advertisements", "Record everything Spotify plays" and "Include
advertisements" were three on/off settings over four combinations — two of which did exactly the
same thing, while two of the switches did nothing at all unless a third allowed it. The card had
to grey controls out to explain a hierarchy that was never visible in the first place, and it
still read as three contradictory answers to one question. It is now one question with one
answer: **Record — recognisable tracks only** (the default), **everything except
advertisements**, or **everything, advertisements included**. Nothing about which files get
recorded has changed; there is simply no longer a way to ask for a combination that means
something other than what it says.
- **"Mute advertisements" never muted anything.** The code behind it had no callers, so the switch
shipped on by default and its one real effect was to silently veto "Record everything Spotify
plays" — turn that on and nothing happened, with nothing on the page to say why. The predecessor
never hit this because its form unchecked mute-ads the moment record-everything was checked;
Offstream had inherited the rule and not the interlock. The setting is stored as
`recording.recordSelection` now and the three keys it replaces are gone, so a settings file
hand-tuned around the old behaviour comes back at the default.
- **The Advanced page's settings sit in the sections they belong to.** What to do about a file that
already exists — and whether to tell Spotify to move on from it — are rules about recording, not
about what recordings are called, and both had been living under **File names**. They have moved
into **Recording**, which is what the **Detection** card is now called: it had long since stopped
being only about detection.
- **Telling Spotify to move on is a fourth answer to "when that file already exists", not a switch
beside it.** It was a separate on/off setting that did nothing under two of the three policies —
overwriting the file and saving a second copy both record the track again, leaving nothing to
move past — so it greyed itself out half the time and needed a sentence explaining why. Six
combinations, four outcomes: the same arithmetic as the advertisement switches above, and the
same fix. The choice is now **keep the one on disk**, **keep it and skip to the next track**,
**replace it**, or **save the new one alongside it**. Its `output.skipAlreadyRecordedTracks` key
is gone and its value lives in `output.existingFilePolicy`, so anyone who had asked Offstream to
move on will need to say so again. Giving the dropdown the row to itself also fixes the switch
label, which was clipped at "Also tell Spotify to mov".
- **A setting's description is a tooltip now.** The sentence under a switch cost a line of height
on a page that has none to spare — the card at the bottom has been clipped three times — and it
said the same thing every time whether or not anyone needed it. Hovering the row shows it
instead. The text is unchanged and still translated, and each control carries it as accessible
help text as well, because a tooltip is the one thing on the page a screen reader cannot see.

### Removed

- **Muting advertisements is off the roadmap rather than unfinished.** It sat in the parity list as
something still owed, but there is no coherent behaviour to owe: the predecessor's version muted
*every other application* on the machine and left Spotify at full volume, to catch video adverts
playing through a separate audio session — so it never silenced the advert either. A recorder
reaching out to mute unrelated applications is a side effect nobody asked for, and with the
detection options now saying plainly that adverts are discarded, silencing them buys nothing.

### Added

- **Offstream can tell Spotify to move on from a track it already has.** Keeping the file on disk
Expand All @@ -20,8 +69,10 @@ phase plan these entries follow.
through the parts that already are. "Also tell Spotify to move on", beside the existing-file
setting on the Advanced page, sends a skip through the Windows media transport controls — the
same command as the keyboard's next-track key, so it needs no account, no scope and no Premium
subscription, and it reaches Spotify while it is minimised to the tray. Off by default, greyed
out under the two policies that record the file again, and asked exactly once per track, because
subscription, and it reaches Spotify while it is minimised to the tray. It is the second of the
four answers to **When that file already exists** on the Advanced page — *keep it, and skip to
the next track* — so it cannot be asked for under a policy that records the file again anyway.
Off unless chosen, and asked exactly once per track, because
Spotify goes on reporting the outgoing song for a moment after it takes the command and a second
skip would land on a song nobody had recorded. It is asked twice per track, though: once
immediately, and again when the metadata lookup lands, since a template built on `{album}`,
Expand All @@ -33,6 +84,17 @@ phase plan these entries follow.
the user had just started. It also stops after fifty tracks in a row and says so: a queue
Offstream already has, left on repeat, has no other reason to ever stop skipping. Recording
something new puts the budget back.
- **The filename presets say they are presets, and there are five of them.** Two unlabelled buttons
under the template box read as actions on what is in it — reset, apply — rather than as layouts
to start from, so they now sit behind a **Presets** caption. Alongside *Artist and title* and
*A folder per album* are **Numbered tracks**, which puts the track number in front of each title
so players and Explorer keep album order; **Albums by year**, which leads the album folder with
the year so an artist's records sort chronologically; and **A folder per day**, which groups by
recording date rather than by album. Each button's tooltip is the path its layout produces —
*Names files as `Artist name\(2026) Album name\04 Track title.mp3`* — rendered by the recorder's
own naming code rather than written out by hand, so an example cannot promise a name the recorder
would not write. A preset is only ever picked for its outcome, and the label has room for a name
but not for a path.

### Fixed

Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,5 @@ Beyond that: ffmpeg argv golden tests, encode-integration tests asserted with ff
- Nullable reference types enabled.
- MVVM via CommunityToolkit.Mvvm source generators; no code-behind logic beyond wiring.
- Inline validation (`INotifyDataErrorInfo`), not modal dialogs.
- **A setting's description is a tooltip, not a line under its label** (decided 2026-08-29). The Advanced page has no `ScrollViewer` and the shell's `MinHeight` promises no setting is ever out of reach, so a line of prose per row is height the page does not have — it has been clipped off the bottom three times. Put the sentence in `ToolTip` on the whole row so hovering anywhere finds it, and in `AutomationProperties.HelpText` on the control, because a tooltip is invisible to a screen reader. The text still lives in `Strings.resx` + `.fr.resx` and is still translated. Labels stay inline.
- User-facing strings live in `Offstream.App/Resources/Strings.resx` (+ `.fr.resx`) with an en/fr key-parity test. Resource **keys are re-keyed for Offstream**; do not carry the predecessor's key names across.
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,15 @@ and sorts where you expect it to.

The rest of the page, briefly:

- **If a file already exists** — skip it, overwrite it, or keep both.
- **Mute advertisements** — on by default; adverts are silenced and never saved.
- **Record everything Spotify plays** — include podcasts and anything else without an artist.
- **Name recordings** — the filename template, with a live preview of the next file. Five presets
sit under it — artist and title, a folder per album, numbered tracks, albums by year, a folder
per day — and each one's tooltip shows the names it would produce before you pick it.
- **Record** — how much of what Spotify plays is worth keeping. *Recognisable tracks only* is the
default: Offstream saves what Spotify reports as an artist and a title and discards the rest.
Widen it to *everything except advertisements* to keep podcasts too, or to *everything,
advertisements included* if you want the lot.
- **When that file already exists** — keep the one on disk, keep it and have Spotify skip to the
next track, replace it, or save the new one alongside it.
- **Stop recording after** — a timer, for recording overnight or for exactly an hour.
- **Number the tracks** — writes the counter into the track-number tag, so players sort recordings
in the order they were made.
Expand Down
22 changes: 20 additions & 2 deletions docs/MODERNIZATION-PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ Two rules the schema must honour:
The acceptance checklist. Everything the app does today.

### Recording
WASAPI loopback capture · device selection · device volume · per-track splitting · minimum recorded length · recording timer (hhmmss) · silence trim start/end · mute ads · record everything (podcast) · record ads · skip/overwrite/duplicate existing · force Spotify to skip recorded track · listen to playback on default device · prevent sleep while recording.
WASAPI loopback capture · device selection · device volume · per-track splitting · minimum recorded length · recording timer (hhmmss) · silence trim start/end · ~~mute ads~~ (**dropped 2026-08-28** — see below) · record everything (podcast) · record ads · skip/overwrite/duplicate existing · force Spotify to skip recorded track · listen to playback on default device · prevent sleep while recording.

### Output
MP3 128/160/256/320 · WAV · Opus · **FLAC (new)** · **AAC (new)** · filename template with tokens and folder support · counter with padding · output path · 260-char budgeting (**plus long-path support, new**).
Expand All @@ -243,6 +243,24 @@ Tabs (**Record / Settings / Advanced**, and **Logs** from 2026-08-14) · console

> The tab *structure* carries over because it works. The **tab labels do not** — the predecessor's "Spy" tab is Offstream's **Record** tab, and its "spy options" are **detection options**. §0 applies to user-visible strings as much as to code.
>

### Finding: "mute ads" is dropped, not deferred (2026-08-28)

The setting shipped as **Mute advertisements**, on by default, and never muted anything: `AudioSessions.SetMute` had no callers. Its only effect was one `!` in `RecordingPolicy.IsRecordUnknownActive`, where it silently vetoed "record everything" — so the option could be turned on and would do nothing, with no indication why. The predecessor guarded against exactly that by unchecking mute-ads whenever record-everything was checked (`frmEspionSpotify.cs:732`); Offstream ported the policy expression and not the interlock.

Restoring it was considered and rejected. There is no single behaviour to restore: the predecessor's mute-ads muted **every other application** and held Spotify at full volume (`MainAudioSession.cs:185-195`), to catch video adverts playing through a separate audio session. It never silenced the advert itself, so its own label was wrong too. Reimplementing that would reach outside Spotify to mute unrelated applications — a side effect no one asked a recorder for.

So the three switches collapse into one choice, `recording.recordSelection`, with the three values they could actually express between them: `KnownTracksOnly` (the default), `EverythingExceptAds`, `Everything`. Of the four combinations the booleans allowed, two were behaviourally identical — which is the arithmetic behind the confusion, and the reason relabelling them was not enough. Session mute stays proven (§ Phase 0) and `AudioSessions.SetMute` stays in Core; nothing in the product depends on it.

The same reduction then applied a second time, on the user's prompt, to the existing-file policy. `output.skipAlreadyRecordedTracks` was a boolean beside a three-valued dropdown that did nothing under two of its three values — six combinations, four outcomes — so it is now a fourth policy, `ExistingFilePolicy.SkipAndMoveOn`. Two notes for anyone extending that enum. `Skip` keeps its name and its zero value because the member names are what `settings.json` stores and `JsonStringEnumConverter` throws on a value it does not recognise, which `SettingsStore.Load` turns into a hard "fix or delete the file"; a rename would fail every existing install to gain a tidier identifier. And the four call sites that asked `== Skip` — one of them negated, in `TrackRecorder.AlreadyOnDisk` — now ask `RecordingSettings.KeepsTheExistingFile` instead: a policy forgotten at a comparison like that does not fail the build or a test, it silently records over a file the user asked to keep.

### Finding: the Advanced page cannot be measured by a test (2026-08-29)

The bottom of that page has now been clipped three times, each caught by a screenshot rather than by the build, because nothing above it scrolls and nothing in the build knows how tall it is. A measuring test was attempted: construct `AdvancedPage` on an STA thread with the three dictionaries `App.xaml` merges, `Measure` at the shell's content width, assert `DesiredSize.Height`. It measures correctly, and the numbers are worth keeping even though the test is not: at the shell's content width of 976, the Advanced page needs **495** units against the Settings page's **536**. Those were measured on 2026-08-29 and are carried forward, not re-taken. Two edits later that day left them standing: the existing-file rework turned a row holding a dropdown and a switch into the same row holding only the dropdown, and the template presets went from two buttons to five in the row that already held them. The presets are the one loose end — they sit in a `WrapPanel`, so at the window's narrowest they may take a second line, which is about **36** units and puts the page at roughly **531**. Still under the Settings page's 536, which is the comparison the fit rests on, but the margin is now that difference rather than forty units. Any edit that adds a row outright invalidates the figure, and there is no test to say so. Settings has never been reported as clipped, so Advanced now fits wherever Settings does — which is the closest thing to a proof available without a shown window. The test itself **cannot be kept**, and the reason is not the STA thread.

`StaticResource` resolves at load time, so the dictionaries must be in place before the page's constructor runs, which means an `Application`. `Application.Current` is a process-wide singleton bound to the thread that created it, and the test's STA thread ends with the test — leaving every later test in the assembly marshalling onto a dead dispatcher. Nine `RecordViewModelTests` failed on the first run for exactly that reason. Measuring the shell instead does not work either: an unshown window arranges to nothing, so the host `ContentControl` reports zero.

This is the empirical form of the note already on `AppServicesTests` ("asserts what is registered, never what resolves"). The page's height stays a hand-checked property until either the suite gains an assembly-scoped STA fixture that owns the `Application` for its lifetime, or the page gains a `ScrollViewer` and stops needing to be measured at all.
> The log pane became a fourth tab on 2026-08-14; see the Phase 7 findings. The predecessor's structure is a starting point, not a ceiling.

### Filename template — behaviour preserved exactly
Expand Down Expand Up @@ -465,7 +483,7 @@ Delivered in four PRs: **PR 1** (shell scaffold, DI, navigation, design tokens),
- ✅ App host, DI, navigation, Fluent theme, dark mode.
- ✅ **Record tab:** status, now-playing, elapsed, console log (with filter + copy), start/stop — **plus a live waveform**, which the plan did not ask for; see below.
- ✅ **Settings tab:** output path, device, quality, min length, format (incl. FLAC/AAC), metadata provider (*and, from 2026-08-12, the Last.fm API key and the Spotify sign-in that make the provider choice mean something*).
- ✅ **Advanced tab:** tray *(setting; the icon itself is below)*, timer, counter, filename template with a token reference **and live preview**, existing-file policy, detection options, tag options.
- ✅ **Advanced tab:** tray *(setting; the icon itself is below)*, timer, counter, filename template with a token reference, **a live preview and five layout presets that show the names they would produce**, existing-file policy, recording options *(the "detection options" card, renamed on 2026-08-29 once it had stopped being only about detection)*, tag options.
- ✅ Inline validation via `INotifyDataErrorInfo` instead of modal dialogs.
- ✅ i18n from `Offstream.App/Resources/Strings.resx` with an en/fr key-parity test (the reference tree's translation test, re-namespaced). **Resource keys are re-keyed for Offstream** — this is the file where inherited naming would otherwise survive longest.
- ✅ Tray icon, minimise behaviour, single-instance guard.
Expand Down
Loading
Loading