Type the cover art as the front cover, settle three profile differences, and delete a guard ffmpeg makes unnecessary

CHANGELOG.md

@@ -51,6 +51,16 @@ phase plan these entries follow.
   or Advanced left the Record page describing the file the previous settings would have produced,
   until something was recorded. It now follows a saved setting straight away.
 
+- **Embedded cover art was filed as "Other", not as the front cover.** A picture inside an audio
+  file carries a type — front cover, back cover, artist photo, and a dozen more — and a
+  description of its own. Offstream set neither. It marked the stream as attached artwork and
+  left the type at 0, which both ID3's `APIC` frame and FLAC's `METADATA_BLOCK_PICTURE` spell
+  `Other`, so software that goes looking for a front cover specifically passed over files that
+  had one: the sleeve was in there, correct and complete, and still showed as a grey box. The
+  picture is now typed as the front cover and described as one. M4A is the exception and always
+  was — its container has nowhere to keep either field, so the sleeve goes in bare there whatever
+  is asked for.
+
 ## [0.2.0] - 2026-08-31
 
 **Upgrading from 0.1.0 resets one setting.** The three advertisement switches

docs/MODERNIZATION-PLAN.md

@@ -173,7 +173,7 @@ All conversion goes through ffmpeg. Capture writes raw PCM WAV to a temp file; f
 | Format | Args | Notes |
 | --- | --- | --- |
 | MP3 | `-c:a libmp3lame -b:a {rate}k` | CBR; VBR (`-q:a`) can be exposed later |
-| WAV | `-c:a pcm_s16le` | Or stream-copy when the temp already matches |
+| WAV | `-c:a pcm_s16le` | Never a stream copy — the temp is float32, see the 2026-09-02 finding |
 | Opus | `-c:a libopus -b:a {rate}k` | Ogg container, `.opus` |
 | **FLAC** | `-c:a flac -compression_level 8` | New — near-free once ffmpeg owns conversion |
 | **AAC/M4A** | `-c:a aac -b:a {rate}k` | New |
@@ -373,7 +373,7 @@ Prove the risky parts survive the move before restructuring anything.
 | Enums (media format, provider, cover size, restrictions, policies) | ✅ ported |
 | `Text/StringExtensions`, `Text/EnumerableExtensions` | ✅ ported |
 | `Spotify/SpotifyWindowTitles` (idle/ad detection) | ✅ ported |
-| `Audio/WaveFormatExtensions` (MP3 limits) | ✅ ported |
+| ~~`Audio/WaveFormatExtensions` (MP3 limits)~~ | **Deleted 2026-09-02** — ported, never called, and ffmpeg makes the check unnecessary; see the encoding-profile finding |
 | `Naming/PathText` (diacritics, segment cleaning, tidy) | ✅ ported |
 | `Naming/FileNameTemplate` | ✅ ported, syntax byte-identical |
 | `Metadata/Track` | ✅ ported, with its own tests |
@@ -1042,6 +1042,77 @@ The tempting fix is worse than the bug: splitting the box on commas the way the
 
 The never-erase rule from the day before became one helper, `LibraryLookup`, rather than two hand-written copies, and grew from genre and year to all seven fields a lookup can leave empty. There are still two call sites, and both are still load-bearing — `MetadataViewModel.FetchOneAsync` for every automatic lookup and `SpotifyCatalogEnricher` for the manual **Use this** path, which does not pass through it — but the rule itself now exists once, so the next field cannot be added to one copy and forgotten in the other. That is precisely how the bug survived its first fix.
 
+### Finding: three of the four ways the encoding profiles differ from a comparable recorder are ours to keep (2026-09-02)
+
+The profiles were audited flag by flag against another Windows Spotify recorder's, on the working
+assumption that any difference was a gap. One was. The other three are decisions this project had
+already taken, and they are written down here rather than left to be rediscovered, because each
+one reads from the outside as an obvious improvement that nobody has got round to.
+
+**The real gap: the attached picture was typed `Other`.** `-disposition:v attached_pic` marks a
+stream as cover art and settles nothing else about it — in particular it leaves the picture type
+at 0, which is `Other` in both ID3's `APIC` frame and FLAC's `METADATA_BLOCK_PICTURE`. Every file
+Offstream had ever written carried its sleeve under that type, and software that looks for a
+front cover specifically skips it.
+
+Two `-metadata:s:v` arguments fix it, and the surprise is that **neither is free text in the way
+it looks**. `comment` is read by the muxer as the picture *type*, matched against the spellings
+the format defines: `Cover (front)` selects type 3, and anything unrecognised falls back to
+`Other` without complaint — encoding with `comment=Sleeve test` produced a file reporting
+`Other`, and the string was nowhere in it. `title` is the description and is genuinely free text.
+Reading the result back with ffprobe is not enough to tell these apart, because ffprobe reports
+the *type* through the same `comment` key the argument uses, so a file where the string was
+stored verbatim and a file where it was interpreted look identical; the isolation run — each
+argument alone, then a byte search of the output — is what separated them, and
+`Encode_TypesTheCoverArtAsTheFrontCover` asserts it through TagLib# for that reason. M4A takes
+both arguments and stores neither: the mov muxer keeps a cover as a bare atom with nowhere to put
+a type or a description.
+
+**MP3 stays constant-bitrate.** The other ladder passes `-abr 1` at every rung and keeps plain
+`-b:a 320k` for a top rung of its own. Offstream's `-b:a {rate}k` with no `-abr` *is* that top
+rung — the default output is already the best MP3 that ladder can produce. Taking the `-abr 1`
+without also adding a constant-bitrate rung would move the default down a notch on the
+most-used format; adding the rung means the bitrate stops being a plain kbps number, which is the
+`LAMEPreset`-shaped setting §5.1 rejected on purpose. ABR is a genuine quality win at 96 and 128
+kbps, so this is worth reopening if those rungs turn out to be the ones people use. It buys
+nothing at a default of 320.
+
+**WAV stays `-c:a pcm_s16le`, and the stream-copy §5.1 used to offer is off the table.** The
+other implementation copies the captured WAV instead of encoding it, which is the same idea. The
+temp file never matches: `WasapiLoopbackCapture` reports the endpoint's shared-mode mix format,
+which on Windows is 32-bit float — `codec_name=pcm_f32le`, confirmed with ffprobe. Copying it
+would make Offstream's WAV output float32, twice the size of the 16-bit file it writes today, in
+the one format people pick precisely because they are handing it to something else. The copy is
+not a cheaper way to produce the same file; it produces a different file.
+
+**Cover art stays `-c:v mjpeg`.** The other implementation pipes the downloaded bytes through
+`-c:v copy`, which is a free generation of quality whenever the source is already JPEG. It is not
+always JPEG here: `CoverArtFetcher.TempFileFor` keeps a `.png` extension deliberately, because
+`CoverArtWriter` derives the picture's MIME type from it, and `copy` would put a PNG into an APIC
+frame. The re-encode is normalisation, not waste.
+
+One thing the audit turned up that is neither, and it is now gone:
+`WaveFormatExtensions.GetMp3Restrictions` had no caller outside its own tests. It answered "which
+MP3 limits does this capture format exceed" — more than two channels, or above 48 kHz — because
+LAME had to be told, and the reference implementation resampled and reduced channels by hand
+before handing it the buffer. ffmpeg does both unasked, so the answer had nowhere to go and the
+method, its `Mp3Restriction` enum and its four ported tests are deleted.
+
+That was checked rather than assumed, because libmp3lame really does refuse more than two
+channels and really is capped at 48 kHz — if ffmpeg had passed the constraint through instead of
+resolving it, the dead guard would have been pointing at a live crash on any 5.1 or high-rate
+endpoint. Encoding a 6-channel 48 kHz float32 WAV and a stereo 96 kHz one through the exact
+profiles this app ships:
+
+| Input | MP3 | FLAC | AAC | Opus |
+| --- | --- | --- | --- | --- |
+| 5.1 @ 48 kHz | downmixed to stereo | 6 channels kept | 6 channels kept | 6 channels kept |
+| stereo @ 96 kHz | resampled to 48 kHz | — | — | — |
+
+Every case exits 0. Worth knowing for its own sake: **a multichannel endpoint survives as
+multichannel in every format except MP3**, which is a real difference between the formats on the
+Settings page and not something the app says anywhere.
+
 ---
 
 ## 12. Risks

src/Offstream.Core/Encoding/FFmpegArguments.cs

@@ -70,6 +70,26 @@ public static IReadOnlyList<string> Build(EncodeRequest request)
             // With two inputs the mapping must be explicit, or ffmpeg picks one stream per type.
             args.AddRange(["-map", "0:a", "-map", "1:v"]);
             args.AddRange(["-c:v", "mjpeg", "-disposition:v", "attached_pic"]);
+
+            // Types the picture and names it. -disposition:v attached_pic only says "this
+            // stream is cover art" — it leaves the picture type at 0, which both ID3's APIC
+            // frame and FLAC's METADATA_BLOCK_PICTURE spell "Other", and software looking for a
+            // front cover specifically then passes over a file that has one.
+            //
+            // The two arguments do different jobs, and neither is free text. "comment" is read
+            // by the muxer as the picture type and only takes the spellings the format defines:
+            // "Cover (front)" selects type 3, and anything it does not recognise silently falls
+            // back to "Other" — so this string is a constant, not a caption. "title" is the
+            // description, and that one is genuinely free text. Both verified by encoding with
+            // each argument alone and reading the result back.
+            //
+            // M4A takes them and stores neither: the mov muxer keeps a cover as a bare atom with
+            // nowhere to put a type or a description. They are still written in the shared branch
+            // rather than per format, because the alternative is an exception list that has to
+            // stay right about a muxer detail nobody re-checks. English on purpose: this is tag
+            // content other software matches on, not a string the user reads.
+            args.AddRange(["-metadata:s:v", "title=Album cover"]);
+            args.AddRange(["-metadata:s:v", "comment=Cover (front)"]);
         }
 
         foreach (var argument in profile.CodecArguments)

src/Offstream.Core/Recording/RecordingEnums.cs

@@ -62,13 +62,6 @@ public enum SilenceTrim
     TrimStart,
 }
 
-/// <summary>Why a wave format cannot be encoded to MP3 without conversion.</summary>
-public enum Mp3Restriction
-{
-    Channel,
-    SampleRate,
-}
-
 /// <summary>UI language.</summary>
 public enum LanguageType
 {

tests/Offstream.Core.Tests/Encoding/CoverArtIntegrationTests.cs

@@ -54,6 +54,46 @@ public async Task Encode_AttachesCoverArtInTheSamePass(MediaFormat format)
         Assert.Contains("attached_pic=1", streams, StringComparison.Ordinal);
     }
 
+    /// <summary>
+    /// The attached picture is typed as the front cover, and described.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <c>-disposition:v attached_pic</c> does not settle this. It marks the stream as cover art
+    /// and leaves the picture type at 0 — <c>Other</c> — which is what every file written before
+    /// 2026-09-02 carries, and what makes software hunting for a front cover skip a file that
+    /// has one. The type comes from the stream's <c>comment</c> tag, which the muxer matches
+    /// against the spellings the format defines rather than storing verbatim.
+    /// </para>
+    /// <para>
+    /// Asserted through TagLib# rather than ffprobe because that is what a player sees, and
+    /// because ffprobe reports the type through the same <c>comment</c> key the argument uses,
+    /// so it would pass on a file where the string had been stored and never interpreted.
+    /// M4A is excluded: the mov muxer has nowhere to keep either field.
+    /// </para>
+    /// </remarks>
+    [Theory]
+    [InlineData(MediaFormat.Mp3)]
+    [InlineData(MediaFormat.Flac)]
+    public async Task Encode_TypesTheCoverArtAsTheFrontCover(MediaFormat format)
+    {
+        var source = await _workspace.CreateSourceWavAsync();
+        var cover = await _workspace.CreateCoverArtAsync();
+        var output = _workspace.PathFor(format, "typed-cover");
+
+        var outcome = await new AudioEncoder(_workspace.Runner).EncodeAsync(
+            new EncodeRequest(source, output, format, 192, SampleTrack(), cover));
+
+        Assert.False(outcome.HasWarning);
+
+        using var tagged = TagLib.File.Create(output);
+
+        var picture = Assert.Single(tagged.Tag.Pictures);
+
+        Assert.Equal(TagLib.PictureType.FrontCover, picture.Type);
+        Assert.Equal("Album cover", picture.Description);
+    }
+
     /// <summary>
     /// <b>The §5.2 fallback.</b> ffmpeg's <c>METADATA_BLOCK_PICTURE</c> support for Ogg is weak,
     /// so the profile routes Opus through TagLib# after the encode. The picture must survive

tests/Offstream.Core.Tests/Encoding/FFmpegArgumentsTests.cs

@@ -122,12 +122,39 @@ public void CoverArt_ForMp3_AddsSecondInputAndExplicitMaps()
             "-i", @"C:\temp\cover.jpg",
             "-map", "0:a", "-map", "1:v",
             "-c:v", "mjpeg", "-disposition:v", "attached_pic",
+            "-metadata:s:v", "title=Album cover",
+            "-metadata:s:v", "comment=Cover (front)",
             "-c:a", "libmp3lame", "-b:a", "320k",
             "-id3v2_version", "3",
             @"C:\music\out.mp3",
         ], argv);
     }
 
+    /// <summary>
+    /// Every container that takes an attached picture also gets the picture's description.
+    /// </summary>
+    /// <remarks>
+    /// The description is a separate field from the disposition, and it is the one a player
+    /// shows when it lists what pictures a file holds. Pinned for all three formats rather than
+    /// for MP3 alone, because it is written once in the shared branch and a format added later
+    /// picks it up silently — this is what says that is intended.
+    /// </remarks>
+    [Theory]
+    [InlineData(MediaFormat.Mp3)]
+    [InlineData(MediaFormat.Flac)]
+    [InlineData(MediaFormat.Aac)]
+    public void CoverArt_NamesThePictureItAttaches(MediaFormat format)
+    {
+        var argv = FFmpegArguments.Build(Request(format, cover: @"C:\temp\cover.jpg")).ToList();
+
+        var disposition = argv.IndexOf("attached_pic");
+        Assert.True(disposition >= 0);
+
+        Assert.Equal(
+            ["-metadata:s:v", "title=Album cover", "-metadata:s:v", "comment=Cover (front)"],
+            argv.GetRange(disposition + 1, 4));
+    }
+
     /// <summary>
     /// The whole point of the ID3 version flag: it travels with the cover art.
     /// </summary>