Skip to content
Closed
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
11 changes: 11 additions & 0 deletions docs/MODERNIZATION-PLAN.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -809,6 +809,17 @@ error paths and the scope list.
- **It is a success-path step, into a gap only.** A provider that found nothing at all leaves a bare recording, and one genre on an otherwise untagged file is not worth a second request; a provider that did supply genres is not second-guessed. A fallback that throws is swallowed at `Debug` — everything else on the track is already correct by then, and losing an album because a genre lookup timed out would be the tail wagging the dog.
- **SMTC cannot help here, verified rather than assumed.** `GlobalSystemMediaTransportControlsSessionMediaProperties` does define a `Genres` field. Probed live against the running Spotify client, it comes back `count=0` while Title, Artist, AlbumTitle, AlbumArtist and TrackNumber are all populated. Reading it would be code that returns nothing.


#### The media session as a metadata floor (2026-08-14)

- **Precedence is API first, media session underneath** (user's call, 2026-08-14). Not the other way round, and not "client first with the API as fallback" — which sounds equivalent and is not. The two sources have asymmetric coverage: SMTC supplies title, artist, album, album artist and track number, and *never* supplies year, release date, genre, album track count, disc or copyright. The API is therefore the only source for half the tag set, so a literal client-first rule would still call it on every track and save nothing. What is worth having is the other direction: the provider wins wherever it answers, and what the client already knew fills the gaps it leaves.
- **The gap it closes is a real one, and it was silent.** When the match guard rejected all four attempts — or no provider was configured, or Spotify was down — the recording was written with nothing. The media session had already reported artist, title, album, album artist and position for that exact track, with more certainty than any lookup, and all of it was being discarded. Now it stands.
- **`SmtcSnapshot` gained `AlbumArtist` and `TrackNumber`**, both optional so the window-title path — which supplies neither — and every existing construction site keep working unchanged. A `TrackNumber` of 0 maps to null: Spotify numbers from 1, so zero means "not reported" rather than a zeroth track. A blank album artist maps to null rather than to an album credited to the empty string, which would otherwise beat a provider that did know.
- **The mappers fill; they no longer clear.** This is the load-bearing half, and it is a deliberate behaviour change in both `SpotifyTrackMapper` and `LastFmTrackMapper`: `Album`, `AlbumArtists` and `AlbumPosition` are only assigned when the provider actually has a value. The `SetArtistFromApi` / `SetTitleFromApi` idiom already worked this way for artist and title — this extends it to the three fields the media session can now supply.
- **One existing test asserted the opposite** and was updated rather than worked around: `ApplyAlbum_WithAnEmptyResponse` pinned that an empty album object wrote `""` and `[]` over whatever was there. That was harmless while the only other source was a window title, which carries no album at all. It stopped being harmless the moment the media session started carrying one, because a provider that could not answer would erase what the client had said for certain.
- **Cover art is deliberately not taken from the media session.** SMTC exposes a thumbnail, and it is much smaller than the 640×640 the Web API returns. As a last resort for a file that would otherwise have no art it would be defensible; as anything that could win over the API's image it is a downgrade users notice in a library. Left out until there is a reason to want it.
- **Genre is not part of this**, because the media session has none to give — probed live, Spotify returns `count=0` for it. See the genre findings above.

**Still to do in this phase:** nothing.

**Exit:** track detection survives Spotify minimised to tray; VB-CABLE presence is detected and its absence degrades to a documented link, with no vendor binaries in the repo pending question 9.
Expand Down
8 changes: 5 additions & 3 deletions src/Offstream.Core/Metadata/Providers/LastFmTrackMapper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,8 +33,10 @@ public static void Apply(Track track, LastFmTrack response)
ArgumentNullException.ThrowIfNull(track);
ArgumentNullException.ThrowIfNull(response);

track.Album = response.Album?.Title;
track.AlbumPosition = response.Album?.TrackPosition;
// Fills, never clears — the media session may already have supplied both, and Last.fm
// not knowing an album is not the same as the track not having one.
track.Album = string.IsNullOrWhiteSpace(response.Album?.Title) ? track.Album : response.Album.Title;
track.AlbumPosition = response.Album?.TrackPosition ?? track.AlbumPosition;

// Last.fm reports milliseconds; everything downstream works in seconds.
track.Length = response.Duration is > 0 ? response.Duration / 1000 : null;
Expand All@@ -46,7 +48,7 @@ public static void Apply(Track track, LastFmTrack response)
string[] albumArtists = track.Artist is null ? [] : [track.Artist];

track.Performers = [.. albumArtists.Concat(track.ToString().ToPerformers())];
track.AlbumArtists = albumArtists;
track.AlbumArtists = albumArtists is { Length: > 0 } ? albumArtists : track.AlbumArtists;
}

/// <summary>
Expand Down
12 changes: 9 additions & 3 deletions src/Offstream.Core/Metadata/Providers/SpotifyTrackMapper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -49,7 +49,9 @@ public static void Apply(Track track, FullTrack spotifyTrack)
track.SetTitleFromApi(SpotifyTitleParser.TagAt(titleTags, 1));
track.SetTitleExtendedFromApi(SpotifyTitleParser.TagAt(titleTags, 2), separatorType);

track.AlbumPosition = PositiveOrNull(spotifyTrack.TrackNumber);
// Fills, never clears. The detected track may already carry a position from the media
// session, and a provider that has none of its own must not take that away — see ToTrack.
track.AlbumPosition = PositiveOrNull(spotifyTrack.TrackNumber) ?? track.AlbumPosition;
track.Performers = performers;
track.Disc = PositiveOrNull(spotifyTrack.DiscNumber);
}
Expand All@@ -66,8 +68,12 @@ public static void Apply(Track track, FullAlbum spotifyAlbum)
ArgumentNullException.ThrowIfNull(track);
ArgumentNullException.ThrowIfNull(spotifyAlbum);

track.AlbumArtists = ArtistNames(spotifyAlbum.Artists);
track.Album = spotifyAlbum.Name;
var albumArtists = ArtistNames(spotifyAlbum.Artists);

// As above: the media session already supplied both for the track being recorded, so an
// album object that is missing either leaves what is there rather than blanking it.
track.AlbumArtists = albumArtists is { Length: > 0 } ? albumArtists : track.AlbumArtists;
track.Album = string.IsNullOrWhiteSpace(spotifyAlbum.Name) ? track.Album : spotifyAlbum.Name;
track.Genres = spotifyAlbum.Genres?.ToArray() ?? [];
track.Year = ParseReleaseYear(spotifyAlbum.ReleaseDate);
track.AlbumArtUrl = ChooseCoverUrl(spotifyAlbum.Images);
Expand Down
33 changes: 31 additions & 2 deletions src/Offstream.Core/Spotify/Smtc/SmtcTrackSource.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,12 +7,29 @@ namespace Offstream.Core.Spotify.Smtc;
/// <param name="Title">The track title.</param>
/// <param name="Album">The album, when Spotify supplied one.</param>
/// <param name="IsPlaying">Whether the session is playing rather than paused or stopped.</param>
/// <param name="AlbumArtist">
/// Who the album is credited to, which differs from <paramref name="Artist"/> on compilations
/// and features — the distinction a library groups by.
/// </param>
/// <param name="TrackNumber">The position on the album, or null when Spotify did not say.</param>
/// <remarks>
/// <para>
/// A snapshot rather than the live WinRT session: it makes the mapping below a pure function of
/// four values, which is what lets every rule in it be tested without a media session, an audio
/// its values, which is what lets every rule in it be tested without a media session, an audio
/// endpoint, or Spotify installed.
/// </para>
/// <para>
/// The last two are optional so that every existing construction site — and the window-title
/// path, which has no such information — keeps working unchanged.
/// </para>
/// </remarks>
public readonly record struct SmtcSnapshot(string? Artist, string? Title, string? Album, bool IsPlaying);
public readonly record struct SmtcSnapshot(
string? Artist,
string? Title,
string? Album,
bool IsPlaying,
string? AlbumArtist = null,
int? TrackNumber = null);

/// <summary>Reads Spotify's media transport session, if it has one.</summary>
public interface ISmtcSessions
Expand DownExpand Up@@ -72,16 +89,28 @@ public sealed class SmtcTrackSource(ISmtcSessions sessions) : ITrackSource
/// A session that is paused is never an ad, whatever it says. The placeholder lingers after
/// playback stops, and treating it as an ad then would suppress the next real track.
/// </para>
/// <para>
/// <b>Album artist and track number are carried even though a provider usually replaces
/// them.</b> They are the floor: when no metadata provider is configured, or when the one
/// that is cannot match the track, these are what the file is tagged with — and they come
/// from the client that is playing the track, so they describe it with certainty rather than
/// with a lookup's confidence. See the mappers, which fill rather than clear.
/// </para>
/// </remarks>
public static Track ToTrack(SmtcSnapshot snapshot)
{
var hasArtist = !string.IsNullOrWhiteSpace(snapshot.Artist);
var albumArtist = Trimmed(snapshot.AlbumArtist);

return new Track
{
Artist = Trimmed(snapshot.Artist),
Title = Trimmed(snapshot.Title),
Album = Trimmed(snapshot.Album),
AlbumArtists = albumArtist is null ? null : [albumArtist],

// Spotify numbers from 1, so a zero means "not reported" rather than a zeroth track.
AlbumPosition = snapshot.TrackNumber is > 0 ? snapshot.TrackNumber : null,
Playing = snapshot.IsPlaying,
Ad = snapshot.IsPlaying && (snapshot.Title.IsAdvertisement() || !hasArtist),
};
Expand Down
4 changes: 3 additions & 1 deletion src/Offstream.Core/Spotify/Smtc/WindowsSmtcSessions.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -52,7 +52,9 @@ public sealed class WindowsSmtcSessions : ISmtcSessions
properties.Artist,
properties.Title,
properties.AlbumTitle,
status == GlobalSystemMediaTransportControlsSessionPlaybackStatus.Playing);
status == GlobalSystemMediaTransportControlsSessionPlaybackStatus.Playing,
properties.AlbumArtist,
properties.TrackNumber);
}

private static GlobalSystemMediaTransportControlsSession? FindSpotify(
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -419,4 +419,82 @@ public async Task EnrichAsync_WithNoArtistId_LeavesGenresEmptyWithoutAsking()
Assert.Empty(track.Genres!);
harness.Artists.Verify(x => x.Get(It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never());
}

// ---- the floor holds when the provider cannot ----

/// <summary>
/// The failure this is really about: the match guard rejecting every attempt used to leave a
/// recording with nothing, even though the media session had already said — with certainty,
/// because it is the client playing the track — what the album and position were.
/// </summary>
[Fact]
public async Task EnrichAsync_WhenNothingMatches_LeavesTheDetectedMetadataIntact()
{
var harness = new Harness();
harness.ReturnsPlayback(new CurrentlyPlaying
{
IsPlaying = true,
Item = PlayingTrack("Some Entirely Different Song"),
});

var track = new Track
{
Artist = "Artist",
Title = "Title",
Album = "Detected Album",
AlbumArtists = ["Detected Album Artist"],
AlbumPosition = 7,
};

var enriched = await harness.Provider.EnrichAsync(track);

Assert.False(enriched);
Assert.Equal("Detected Album", track.Album);
Assert.Equal(["Detected Album Artist"], track.AlbumArtists!);
Assert.Equal(7, track.AlbumPosition);
}

/// <summary>An API fault is the same story: tags degrade to what the client already knew.</summary>
[Fact]
public async Task EnrichAsync_WhenSpotifyFails_LeavesTheDetectedMetadataIntact()
{
var harness = new Harness();
harness.Fails(HttpStatusCode.InternalServerError, "Spotify is having a moment.");

var track = new Track
{
Artist = "Artist",
Title = "Title",
Album = "Detected Album",
AlbumArtists = ["Detected Album Artist"],
AlbumPosition = 7,
};

await harness.Provider.EnrichAsync(track);

Assert.Equal("Detected Album", track.Album);
Assert.Equal(["Detected Album Artist"], track.AlbumArtists!);
Assert.Equal(7, track.AlbumPosition);
}

/// <summary>
/// A partial answer fills its gaps from the floor rather than blanking them: Spotify knew the
/// album but reported no position, so the media session's position stands.
/// </summary>
[Fact]
public async Task EnrichAsync_WithAnAnswerThatHasNoPosition_KeepsTheDetectedOne()
{
var harness = new Harness();
var spotifyTrack = PlayingTrack("Title");
spotifyTrack.TrackNumber = 0; // The SDK's "not populated".

harness.ReturnsPlayback(new CurrentlyPlaying { IsPlaying = true, Item = spotifyTrack });
harness.ReturnsAlbum("album-1", new FullAlbum { Name = "Real Album", Genres = [], Images = [] });

var track = new Track { Artist = "Artist", Title = "Title", AlbumPosition = 7 };
await harness.Provider.EnrichAsync(track);

Assert.Equal("Real Album", track.Album);
Assert.Equal(7, track.AlbumPosition);
}
}
69 changes: 66 additions & 3 deletions tests/Offstream.Core.Tests/Metadata/SpotifyTrackMapperTests.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -114,21 +114,84 @@ public void ApplyTrack_SplitsTheTitleExactlyAsTheWindowTitleParserDoes(
Assert.Equal(expectedExtended, track.TitleExtended);
}

/// <summary>
/// An empty album object leaves the album fields alone rather than blanking them.
/// </summary>
/// <remarks>
/// This asserted the opposite until 2026-08-14 — that an empty response wrote <c>""</c> and
/// an empty array over whatever was there. That was harmless while the only other source was
/// a window title, which supplies no album at all; it stopped being harmless when the media
/// session began supplying album, album artist and track number, because a provider that
/// could not answer would erase what the client had already said for certain.
/// </remarks>
[Fact]
public void ApplyAlbum_WithAnEmptyResponse_ReturnsExpectedTrack()
public void ApplyAlbum_WithAnEmptyResponse_LeavesTheAlbumFieldsAlone()
{
var fullAlbum = new FullAlbum { Artists = [], Name = "", Genres = [], Images = [] };

var track = WindowTitleTrack();
SpotifyTrackMapper.Apply(track, fullAlbum);

Assert.Equal([], track.AlbumArtists!);
Assert.Equal("", track.Album);
Assert.Null(track.AlbumArtists);
Assert.Null(track.Album);
Assert.Equal([], track.Genres!);
Assert.Null(track.Year);
Assert.Null(track.AlbumArtUrl);
}

/// <summary>
/// The floor: what the media session already established survives a provider that has
/// nothing of its own to say about it.
/// </summary>
[Fact]
public void ApplyAlbum_WithAnEmptyResponse_KeepsWhatTheMediaSessionSupplied()
{
var track = new Track
{
Artist = "Artist",
Title = "Title",
Album = "Detected Album",
AlbumArtists = ["Detected Album Artist"],
AlbumPosition = 7,
};

SpotifyTrackMapper.Apply(track, new FullTrack());
SpotifyTrackMapper.Apply(track, new FullAlbum { Artists = [], Name = "", Genres = [], Images = [] });

Assert.Equal("Detected Album", track.Album);
Assert.Equal(["Detected Album Artist"], track.AlbumArtists!);
Assert.Equal(7, track.AlbumPosition);
}

/// <summary>A provider that does know better still wins, which is the whole precedence.</summary>
[Fact]
public void ApplyAlbum_WhenTheProviderKnowsBetter_OverwritesWhatWasDetected()
{
var track = new Track
{
Artist = "Artist",
Title = "Title",
Album = "Detected Album",
AlbumArtists = ["Detected Album Artist"],
AlbumPosition = 7,
};

SpotifyTrackMapper.Apply(track, new FullTrack { Name = "Title", TrackNumber = 3 });
SpotifyTrackMapper.Apply(
track,
new FullAlbum
{
Artists = [new SimpleArtist { Name = "Real Album Artist" }],
Name = "Real Album",
Genres = [],
Images = [],
});

Assert.Equal("Real Album", track.Album);
Assert.Equal(["Real Album Artist"], track.AlbumArtists!);
Assert.Equal(3, track.AlbumPosition);
}

[Fact]
public void ApplyAlbum_WithNoImages_LeavesCoverArtUrlNull()
{
Expand Down
Loading
Loading