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
11 changes: 7 additions & 4 deletions src/Sendspin.SDK/Client/ArtworkEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public sealed class ArtworkReceivedEventArgs : EventArgs
public long Timestamp { get; }

/// <summary>
/// Encoded image bytes (JPEG/PNG/BMP).
/// Encoded image bytes (JPEG/PNG).
/// </summary>
public byte[] ImageData { get; }

Expand All @@ -32,8 +32,9 @@ public ArtworkReceivedEventArgs(int channel, long timestamp, byte[] imageData)
}

/// <summary>
/// A single artwork channel was cleared (an empty binary artwork message: type byte + timestamp,
/// no image data).
/// A single artwork channel was cleared: either by an empty binary artwork message (type byte +
/// timestamp, no image data), or by a <c>stream/end</c> that ends the artwork role while the
/// channel still shows an image.
/// </summary>
public sealed class ArtworkClearedEventArgs : EventArgs
{
Expand All @@ -43,7 +44,9 @@ public sealed class ArtworkClearedEventArgs : EventArgs
public int Channel { get; }

/// <summary>
/// Server clock timestamp in microseconds carried by the clear message.
/// Server clock timestamp in microseconds carried by the clear message. A clear raised for a
/// <c>stream/end</c> has no message of its own, and carries the timestamp of the image it
/// clears instead.
/// </summary>
public long Timestamp { get; }

Expand Down
27 changes: 27 additions & 0 deletions src/Sendspin.SDK/Client/ClientCapabilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,33 @@ internal void ValidateVisualizerRoleSupport()
VisualizerRoleSupport?.Validate();
}

/// <summary>
/// Rejects a player client that advertises no audio formats: the spec requires a player's
/// <c>supported_formats</c> to list at least one entry (spec PR #257).
/// </summary>
/// <remarks>
/// Gated on the player role for the same reason <see cref="ValidateVisualizerRoleSupport"/>
/// is gated on the visualizer role: <c>supported_formats</c> is only put in <c>client/hello</c>
/// when the player role is advertised, so a non-player client that clears the list has nothing
/// to violate. Matched by family (any <c>player@</c> version), the way the hello builder gates
/// the object, so a future <c>player@v2</c> is caught too. <see cref="BufferCapacity"/> also
/// derives from the list, so an empty one would leave the advertised buffer with no format to
/// size against.
/// </remarks>
/// <exception cref="ArgumentException">
/// <see cref="Roles"/> advertises the player role but <see cref="AudioFormats"/> is empty.
/// </exception>
internal void ValidateAudioFormats()
{
if (Roles.Any(r => r.StartsWith("player@", StringComparison.Ordinal)) && AudioFormats.Count == 0)
{
throw new ArgumentException(
"ClientCapabilities.Roles advertises the player role but AudioFormats is empty; "
+ "the player role requires supported_formats to list at least one audio format.",
nameof(AudioFormats));
}
}

/// <summary>
/// Rejects a custom role advertised without an explicit version. The spec (template.md)
/// requires an application-specific role — one whose name is prefixed with <c>_</c> — to
Expand Down
7 changes: 3 additions & 4 deletions src/Sendspin.SDK/Client/IOutputDelayStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,11 @@ public interface IOutputDelayStore

/// <summary>
/// Persists the output delay in milliseconds. Called whenever the delay changes (e.g. an
/// inbound <c>set_output_delay</c> command or a GroupSync calibration offset).
/// inbound <c>set_output_delay</c> command).
/// </summary>
/// <param name="outputDelayMs">
/// The output delay to persist, in milliseconds. May be negative when sourced from a GroupSync
/// calibration offset (which schedules audio later); the <c>set_output_delay</c> command path is
/// always non-negative. Store and round-trip the value as-is.
/// The output delay to persist, in milliseconds, within the spec's <c>output_delay_ms</c> range
/// of 0-5000 (the SDK clamps the applied value before persisting). Store and round-trip it as-is.
/// </param>
void Save(double outputDelayMs);
}
79 changes: 72 additions & 7 deletions src/Sendspin.SDK/Client/MediaDisplayScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,22 @@ internal sealed class MediaDisplayScheduler : IDisposable
/// </summary>
private readonly PendingArtwork?[] _artwork = new PendingArtwork?[ArtworkChannelCount];

/// <summary>
/// The server timestamp of the image each channel is currently displaying, or null when the
/// channel shows nothing — set when an image is raised and cleared when a clear is. Lets a
/// <c>stream/end</c> flush tell apps to clear the channels that still show something (spec
/// #266). Guarded by <see cref="_lock"/>.
/// </summary>
private readonly long?[] _shownArtwork = new long?[ArtworkChannelCount];

/// <summary>
/// Counts <see cref="FlushArtwork"/> calls. The loop takes due artwork out of its slot before
/// raising it outside <see cref="_lock"/>, so a flush in between cannot reach it there; the
/// loop compares this against the value it took the artwork under and drops what a flush has
/// since overtaken. Guarded by <see cref="_lock"/>.
/// </summary>
private int _artworkFlushGeneration;

/// <summary>
/// One pending update per <see cref="ScheduledStateRole"/>, indexed by the enum. The roles
/// are independent: a scheduled <c>color</c> update never displaces a scheduled
Expand Down Expand Up @@ -275,6 +291,7 @@ internal void SubmitArtwork(ArtworkChunk chunk)
if (displayTime <= now && !HasPendingArtworkLocked() && !_dispatching)
{
raiseNow = pending;
_shownArtwork[channel] = pending.ImageData.Length == 0 ? null : pending.ServerTimestamp;
}
else
{
Expand Down Expand Up @@ -391,16 +408,47 @@ internal void FlushVisualizer()
}

/// <summary>
/// Discards pending artwork, leaving visualizer frames held. For a <c>stream/end</c> that
/// names the <c>artwork</c> role — including the routine case of a server dropping that role
/// alone, where artwork already sent for a coming track must not surface but the visualizer
/// stream plays on.
/// Discards pending artwork, leaving visualizer frames held. For a <c>stream/end</c> or
/// <c>stream/clear</c> that names the <c>artwork</c> role — including the routine case of a
/// server dropping that role alone, where artwork already sent for a coming track must not
/// surface but the visualizer stream plays on.
/// </summary>
internal void FlushArtwork()
/// <param name="raiseCleared">
/// True for a <c>stream/end</c>, which is playback termination: after discarding what is
/// pending, raise <c>ArtworkCleared</c> for every channel still showing an image so apps blank
/// their display (spec #266). False for a <c>stream/clear</c> seek, which keeps the current
/// image and only drops what was buffered ahead.
/// </param>
internal void FlushArtwork(bool raiseCleared)
{
List<ArtworkClearedEventArgs>? cleared = null;

lock (_lock)
{
Array.Clear(_artwork);
_artworkFlushGeneration++;

if (raiseCleared)
{
for (int channel = 0; channel < _shownArtwork.Length; channel++)
{
if (_shownArtwork[channel] is { } timestamp)
{
_shownArtwork[channel] = null;
(cleared ??= new()).Add(new ArtworkClearedEventArgs(channel, timestamp));
}
}
}
}

// Raised outside the lock, as the inline artwork path is, so a subscriber cannot re-enter
// the scheduler under it.
if (cleared is not null)
{
foreach (var args in cleared)
{
_raiseArtworkCleared(args);
}
}
}

Expand Down Expand Up @@ -565,20 +613,22 @@ private async Task RunAsync(CancellationToken cancellationToken)
while (!cancellationToken.IsCancellationRequested)
{
int waitMilliseconds;
int artworkFlushGeneration;
bool hasDue;

lock (_lock)
{
long now = _timer.GetCurrentTimeMicroseconds();
TakeDueLocked(now, dueFrames, dueArtwork, dueState);
artworkFlushGeneration = _artworkFlushGeneration;
hasDue = dueFrames.Count > 0 || dueArtwork.Count > 0 || dueState.Count > 0;
_dispatching = hasDue;
waitMilliseconds = WaitMillisecondsLocked(now);
}

if (hasDue)
{
DispatchDue(dueFrames, dueArtwork, dueState);
DispatchDue(dueFrames, dueArtwork, dueState, artworkFlushGeneration);
dueFrames.Clear();
dueArtwork.Clear();
dueState.Clear();
Expand Down Expand Up @@ -691,7 +741,8 @@ private int WaitMillisecondsLocked(long now)
private void DispatchDue(
List<PendingFrame> dueFrames,
List<PendingArtwork> dueArtwork,
List<PendingStateUpdate> dueState)
List<PendingStateUpdate> dueState,
int artworkFlushGeneration)
{
// State first, so a subscriber reacting to the artwork of a track change already sees
// the metadata and colors that image belongs to. See the class remarks.
Expand All @@ -707,6 +758,20 @@ private void DispatchDue(

foreach (var artwork in dueArtwork)
{
lock (_lock)
{
// A flush since this image was taken has discarded it, and a stream/end has
// already told the app to clear: raising it now would leave it on display.
if (_artworkFlushGeneration != artworkFlushGeneration)
{
break;
}

// Recorded as shown only now that it is being raised, so a flush clears exactly
// the channels the app has been given an image for.
_shownArtwork[artwork.Channel] = artwork.ImageData.Length == 0 ? null : artwork.ServerTimestamp;
}
Comment on lines +772 to +773

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declined: what remains is the few microseconds between releasing the lock and calling the handler, and closing it means holding a lock across application callbacks. Worst case is a stale image until the next one arrives.


SafeRaise(_raisePendingArtwork, artwork, "artwork");
}
}
Expand Down
Loading
Loading