Start recording when playback starts, not only when the track changes

CHANGELOG.md

@@ -274,6 +274,19 @@ phase plan these entries follow.
 
 ### Fixed
 
+- **Pressing record with Spotify paused, then pressing play, recorded nothing.** The level meter
+  moved, the counter ran and the page named the song, but no file was ever written until the user
+  pressed stop and start again. Being played is half of what makes a track recordable, and that
+  check ran only when the track changed — while two observations of the same song count as the
+  same track whether or not it is playing, by design, so that a pause mid-song does not read as a
+  new one. A song that started playing therefore raised nothing the session was listening for: it
+  had already passed the track over once as not recordable, and went on waiting for a change that
+  had happened. The predecessor never met this because it waited for Spotify to produce audio
+  before it began watching at all; Offstream starts listening the moment record is pressed, which
+  is worth keeping. The check now also runs when playback starts, for the track already showing
+  and only when nothing is being recorded — a pause mid-song still leaves the recorder alone, and
+  starting with music already playing still produces exactly one recording rather than one that is
+  immediately torn down and discarded as too short.
 - **Every track after the first showed nothing but the artist and title.** The album, cover art
   and destination arrived a second into each song and were gone a moment later, on every track
   except the first one of a session — while the files themselves were tagged correctly, so the

src/Offstream.Core/Recording/RecordingSession.cs

@@ -419,11 +419,49 @@ private void OnTrackTimeChanged(object? sender, TrackTimeChangedEventArgs e)
             TimeSpan.FromSeconds(e.TrackTimeSeconds));
     }
 
-    private void OnPlayStateChanged(object? sender, PlayStateChangedEventArgs e) =>
+    /// <summary>
+    /// Playback started or stopped. Starting is also a chance to record what is already on.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <b>A track is only admitted while it is playing</b> — <see cref="Track.IsNormalPlaying"/>
+    /// is half of <see cref="RecordingPolicy.IsTypeAllowed"/> — and the admission check used to
+    /// run in <see cref="OnTrackChanged"/> alone. <see cref="Track.Equals"/> ignores the play
+    /// state, so a song that goes from paused to playing is not a new track and raised nothing
+    /// but this event. Pressing record while Spotify was paused and then pressing play therefore
+    /// left the session waiting for a track change that had already happened: the meter moved,
+    /// the counter ran, and nothing was ever written until the user stopped and started again.
+    /// The predecessor never met this because it blocked on Spotify producing audio before it
+    /// began watching at all; Offstream starts listening immediately, and pays for it here.
+    /// </para>
+    /// <para>
+    /// Only when nothing is being recorded. A pause mid-song leaves the recorder alone — what
+    /// Spotify stops sending is silence, and the trim handles it — so a resume with a recorder
+    /// still running is the ordinary case and must not start a second one.
+    /// </para>
+    /// </remarks>
+    private void OnPlayStateChanged(object? sender, PlayStateChangedEventArgs e)
+    {
         Report(
             e.Playing ? RecordingStage.Recording : RecordingStage.WaitingForTrack,
             message: e.Playing ? null : "Playback paused.");
 
+        if (!e.Playing || CurrentTrack is not null) return;
+
+        // Not when the recording timer has already elapsed: the session is winding down, and the
+        // next thing it should do is stop rather than take on another track.
+        if (_stopAfterCurrentTrack) return;
+
+        // Only for the track already known. A single poll can see both a new song and a change of
+        // play state — starting from a stopped Spotify does exactly that — and the track change
+        // is raised straight after this, with the stop-the-outgoing-recorder handling that this
+        // path deliberately lacks. Admitting the same track from both would start a recorder only
+        // for the other to tear it down a moment later and discard the fragment as too short.
+        // The poller has not yet stored this observation, so its current track is the previous
+        // one; equality ignores the play state, which is precisely the comparison wanted here.
+        if (e.Track.Equals(_poller.CurrentTrack)) Consider(e.Track);
+    }
+
     /// <summary>
     /// The centre of the session: one track ends, the next begins.
     /// </summary>
@@ -449,8 +487,20 @@ private void OnTrackChanged(object? sender, TrackChangedEventArgs e)
             return;
         }
 
-        var track = e.NewTrack;
+        Consider(e.NewTrack);
+    }
 
+    /// <summary>
+    /// Records <paramref name="track"/> if the settings allow it, and says why when they do not.
+    /// </summary>
+    /// <remarks>
+    /// Shared by the two moments a track can become recordable: it changed, or it started
+    /// playing. Everything specific to a track ending — stopping the outgoing recorder, honouring
+    /// a recording timer that elapsed — stays with <see cref="OnTrackChanged"/>, because neither
+    /// is true of a song resuming.
+    /// </remarks>
+    private void Consider(Track track)
+    {
         if (!_policy.IsTypeAllowed(track))
         {
             Report(RecordingStage.WaitingForTrack, track, message: DescribeSkipped(track));

src/Offstream.Core/Spotify/SpotifyPoller.cs

@@ -124,7 +124,7 @@ public async Task PollOnceAsync(CancellationToken cancellationToken = default)
                 if (_playing) ResumeClock();
                 else PauseClock();
 
-                PlayStateChanged?.Invoke(this, new PlayStateChangedEventArgs(newest.Playing));
+                PlayStateChanged?.Invoke(this, new PlayStateChangedEventArgs(newest.Playing, newest));
             }
 
             var isSameTrack = newest.Equals(previous);

src/Offstream.Core/Spotify/SpotifyPollerEvents.cs

@@ -12,9 +12,18 @@ public sealed class TrackChangedEventArgs(Track? oldTrack, Track newTrack) : Eve
 }
 
 /// <summary>Playback started or stopped.</summary>
-public sealed class PlayStateChangedEventArgs(bool playing) : EventArgs
+/// <param name="playing">Whether Spotify is playing now.</param>
+/// <param name="track">
+/// What it is playing, as this observation saw it. Carried on the event because
+/// <see cref="SpotifyPoller.CurrentTrack"/> is not updated until the poll finishes — a handler
+/// reading it here would get the previous observation, whose play state is the one that just
+/// stopped being true.
+/// </param>
+public sealed class PlayStateChangedEventArgs(bool playing, Track track) : EventArgs
 {
     public bool Playing { get; } = playing;
+
+    public Track Track { get; } = track;
 }
 
 /// <summary>The elapsed position within the current track changed.</summary>

tests/Offstream.Core.Tests/Recording/RecordingSessionTests.cs

@@ -209,6 +209,10 @@ public Harness(
         public static Track Playing(string artist, string title) =>
             new() { Artist = artist, Title = title, Playing = true };
 
+        /// <summary>The same song, showing in Spotify but stopped.</summary>
+        public static Track Paused(string artist, string title) =>
+            new() { Artist = artist, Title = title, Playing = false };
+
         /// <summary>
         /// Changes what Spotify reports. The session owns the poller, so the running poll loop
         /// picks this up on its own — driving <c>PollOnceAsync</c> by hand here would race with it.
@@ -746,6 +750,58 @@ await WaitFor(
             r => r.ConcernsNowPlaying && r.Track is not null && r.Track != r.NowPlaying);
     }
 
+    /// <summary>
+    /// Pressing record with Spotify paused, then pressing play, has to start recording.
+    /// </summary>
+    /// <remarks>
+    /// Playing is half of what makes a track recordable, and the check ran only when the track
+    /// changed. Since <see cref="Track.Equals"/> ignores the play state, a song that starts
+    /// playing is not a new track — so the session skipped it once as not recordable and then
+    /// waited for a change that never came. What the user saw was a level meter moving, a counter
+    /// running, and no file, until they stopped and started again.
+    /// </remarks>
+    [Fact]
+    public async Task Session_WhenPlaybackStartsOnTheTrackAlreadyShowing_RecordsIt()
+    {
+        await using var harness = new Harness();
+
+        harness.Session.Start();
+        harness.Play(Harness.Paused("Artist", "Title"));
+
+        await WaitFor(
+            () => harness.Reports.Any(r => r.Message?.Contains("Not a recordable track", StringComparison.Ordinal) == true),
+            "the paused track to be passed over");
+
+        Assert.Null(harness.Session.CurrentTrack);
+
+        harness.Play(Harness.Playing("Artist", "Title"));
+
+        await WaitFor(
+            () => harness.Session.CurrentTrack?.Title == "Title",
+            "the recorder to start once playback begins");
+    }
+
+    /// <summary>
+    /// And exactly one recorder for it. Starting with Spotify already playing is a single poll
+    /// that sees both a new track and a change of play state, and both used to be a reason to
+    /// start — the second one tearing down the first and discarding the fragment as too short.
+    /// </summary>
+    [Fact]
+    public async Task Session_WhenPlaybackIsAlreadyUnderWay_StartsOneRecorder()
+    {
+        await using var harness = new Harness();
+
+        harness.Session.Start();
+
+        await RecordTrackAsync(harness, Harness.Playing("Artist", "Title"));
+
+        await harness.Session.StopAsync();
+
+        await WaitFor(() => !harness.Recorded.IsEmpty, "the recording to finish");
+
+        Assert.Single(harness.Recorded);
+    }
+
     [Fact]
     public async Task Level_MeasuresWhatCaptureDelivers()
     {