diff --git a/DeepDrftAPI/Controllers/TrackController.cs b/DeepDrftAPI/Controllers/TrackController.cs index bbfc13c..6932d93 100644 --- a/DeepDrftAPI/Controllers/TrackController.cs +++ b/DeepDrftAPI/Controllers/TrackController.cs @@ -705,9 +705,10 @@ public class TrackController : ControllerBase // Streams the track's audio bytes with HTTP Range support. The optional `format` selector (Phase 18.3) // picks the delivery rendering: absent or unrecognized ⇒ Lossless (byte-identical to pre-Phase-18 — // the existing zero-copy disk-stream path, untouched); `opus` ⇒ the derived Ogg Opus 320 artifact - // when present, falling back to lossless when it is not (C2 — never 404/silence). The Opus path serves - // the resolved in-memory bytes via File(..., enableRangeProcessing: true) so Range: bytes=X- still - // yields 206 (load-bearing for streaming + seek), matching the lossless disk-stream's range behavior. + // when present, falling back to lossless when it is not (C2 — never 404/silence). The Opus path streams + // the resolved artifact from a seekable disk FileStream via File(..., enableRangeProcessing: true) — + // no whole-file byte[] — so Range: bytes=X- still yields 206 (load-bearing for streaming + seek), + // matching the lossless disk-stream's range behavior. [HttpGet("{trackId}")] public async Task GetTrack(string trackId, [FromQuery] string? format = null) { @@ -777,13 +778,16 @@ public class TrackController : ControllerBase } // The ?format=opus arm of GetTrack. Resolves the Opus artifact (or the lossless fallback when none - // exists, C2) via TrackFormatResolver and serves the resolved bytes with explicit range processing. - // enableRangeProcessing:true is the load-bearing detail the 18.2 reviewer flagged: File(byte[], ...) - // does NOT get ASP.NET's automatic range handling unless asked, so without this flag a Range: bytes=X- - // would silently return the whole body as 200 instead of a 206 slice — breaking seek for the Opus path - // (and Phase 21 windowing). The resolver reports the *actually-served* format via ResolvedAudio, so the - // content-type matches the bytes (audio/ogg on a hit, the source MIME on a fallback) and the eventual - // client decoder dispatches correctly. + // exists, C2) via TrackFormatResolver and streams the resolved bytes from a seekable disk FileStream — + // never a whole-file byte[] (a ~220 MB Opus / ~970 MB lossless managed allocation per request was the + // read-path OOM defect this closes). enableRangeProcessing:true is load-bearing: the seekable FileStream + // lets ASP.NET honour Range: bytes=X- with a 206 slice (seek + Phase 21 windowing). The resolver reports + // the *actually-served* format via ResolvedAudio, so the content-type matches the bytes (audio/ogg on a + // hit, the source MIME on a fallback) and the eventual client decoder dispatches correctly. + // + // Disposal mirrors the lossless GetTrack path exactly: File() takes ownership of the stream on success + // and disposes it after the response; the inner try disposes ResolvedAudio (and its FileStream) on any + // pre-handoff throw so a handle never leaks. private async Task GetTrackOpusAsync(string trackId) { try @@ -795,11 +799,27 @@ public class TrackController : ControllerBase return NotFound(); } + string contentType; + long streamLength; + Stream innerStream; + try + { + contentType = resolved.ContentType; + // Length from the seekable FileStream — a metadata read, not a body load. + streamLength = resolved.Stream.Length; + innerStream = resolved.Stream; + } + catch + { + await resolved.DisposeAsync(); + throw; + } + _logger.LogInformation( "Streaming track {TrackId} as {Format} ({Size} bytes, {ContentType})", - trackId, resolved.ResolvedFormat, resolved.Audio.Buffer.Length, resolved.ContentType); + trackId, resolved.ResolvedFormat, streamLength, contentType); - return File(resolved.Audio.Buffer, resolved.ContentType, enableRangeProcessing: true); + return File(innerStream, contentType, enableRangeProcessing: true); } catch (Exception ex) { diff --git a/DeepDrftContent/Processors/Opus/ResolvedAudio.cs b/DeepDrftContent/Processors/Opus/ResolvedAudio.cs index 85b3813..d1723e8 100644 --- a/DeepDrftContent/Processors/Opus/ResolvedAudio.cs +++ b/DeepDrftContent/Processors/Opus/ResolvedAudio.cs @@ -1,23 +1,36 @@ -using DeepDrftContent.FileDatabase.Models; using DeepDrftModels.Enums; namespace DeepDrftContent.Processors.Opus; /// /// The outcome of resolving a track + requested to a concrete artifact -/// (Phase 18.2). Carries the bytes, the content-type that matches what was actually returned, -/// and the format actually served — which may differ from the requested one when the C2 fallback fires -/// (Opus requested, no Opus artifact → the lossless artifact + its content-type). The delivery layer -/// (18.3) sets the response Content-Type from so the eventual decoder -/// picks the right decoder for the bytes it receives, not the bytes the listener asked for. +/// (Phase 18.2; read-path streaming). Carries an open, seekable, disk-backed +/// over the artifact's bytes — never a buffered byte[], so a ~220 MB Opus file or ~970 MB lossless +/// source is never materialized in a managed array per request. Also carries the content-type that matches +/// what was actually returned, and the format actually served — which may differ from the requested +/// one when the C2 fallback fires (Opus requested, no Opus artifact → the lossless artifact + its +/// content-type). The delivery layer (18.3) sets the response Content-Type from +/// so the eventual decoder picks the right decoder for the bytes it receives. +/// +/// Ownership: the resolver opens the stream; the caller takes ownership. On the success path the caller hands +/// to File(..., enableRangeProcessing: true), which disposes it after the +/// response. On any pre-handoff throw the caller disposes this instance (which disposes the stream) so the +/// underlying never leaks — mirroring the lossless disk-stream path's catch-path +/// disposal. +/// /// -/// The resolved audio artifact (never null when a resolution succeeds). -/// The MIME type of (e.g. audio/ogg for Opus, -/// or the source's real MIME for lossless). +/// An open, seekable, disk-backed stream over the resolved artifact. The caller owns it. +/// The MIME type of the bytes in (e.g. audio/ogg +/// for Opus, or the source's real MIME for lossless). /// The format actually returned. Equal to the requested format on a direct /// hit; when an Opus request fell back. -public sealed record ResolvedAudio(AudioBinary Audio, string ContentType, AudioFormat ResolvedFormat) +public sealed record ResolvedAudio(Stream Stream, string ContentType, AudioFormat ResolvedFormat) + : IDisposable, IAsyncDisposable { /// True when an Opus request was served the lossless artifact because no Opus existed (C2). public bool DidFallBack(AudioFormat requested) => requested != ResolvedFormat; + + public void Dispose() => Stream.Dispose(); + + public ValueTask DisposeAsync() => Stream.DisposeAsync(); } diff --git a/DeepDrftContent/Processors/Opus/TrackFormatResolver.cs b/DeepDrftContent/Processors/Opus/TrackFormatResolver.cs index bf75d70..5e01175 100644 --- a/DeepDrftContent/Processors/Opus/TrackFormatResolver.cs +++ b/DeepDrftContent/Processors/Opus/TrackFormatResolver.cs @@ -13,11 +13,17 @@ namespace DeepDrftContent.Processors.Opus; /// Downstream waves call this — 18.3 wires it behind the ?format= stream param and serves the /// sidecar over HTTP; this wave delivers only the seam, not the HTTP surface. /// -/// Additive and non-breaking (C2): the lossless branch reads the source exactly as the existing stream -/// path does (via ), and an Opus request for a track -/// with no Opus artifact falls back to lossless rather than failing. Mirrors the -/// derived-artifact lookup precedent: read from the dedicated vault, -/// swallow misses to null (FileDatabase convention), let the caller decide. +/// Additive and non-breaking (C2): the lossless branch streams the source exactly as the existing read +/// path does (via , a non-buffering disk +/// stream), and an Opus request for a track with no Opus artifact falls back to lossless rather than +/// failing. Mirrors the derived-artifact lookup precedent: read from +/// the dedicated vault, swallow misses to null (FileDatabase convention), let the caller decide. +/// +/// +/// Read-path streaming: artifacts are resolved as open, seekable, disk-backed +/// handles — never whole-file byte[] loads — so the delivery layer streams them straight to the +/// response (Range/206 honoured by the seekable FileStream) without buffering a ~220 MB Opus file +/// or a ~970 MB lossless source per request. /// /// public sealed class TrackFormatResolver @@ -48,10 +54,16 @@ public sealed class TrackFormatResolver { if (requestedFormat == AudioFormat.Opus) { - var opus = await _fileDatabase.LoadResourceAsync( - VaultConstants.TrackOpus, OpusTranscodeService.OpusAudioKey(entryKey)); - if (opus is not null) - return new ResolvedAudio(opus, MimeTypeExtensions.GetMimeType(opus.Extension), AudioFormat.Opus); + var opusVault = _fileDatabase.GetVault(VaultConstants.TrackOpus); + if (opusVault is not null) + { + // Disk-backed, seekable stream over the Opus artifact — no whole-file buffer. The caller + // owns the stream (hands it to File(...) on success, disposes on a pre-handoff throw). + var opus = await opusVault.GetEntryStreamAsync(OpusTranscodeService.OpusAudioKey(entryKey)); + if (opus is not null) + return new ResolvedAudio( + opus.Stream, MimeTypeExtensions.GetMimeType(opus.Extension), AudioFormat.Opus); + } // C2 fallback: no Opus artifact yet (legacy row, not backfilled, or transcode failed). Degrade // to lossless rather than 404 — Opus is strictly additive; its absence never means "no audio". @@ -63,16 +75,18 @@ public sealed class TrackFormatResolver } /// - /// Resolves the lossless source artifact and its real MIME — the existing read path, unchanged. Shared - /// by the explicit-lossless branch and the Opus fallback so both produce identical bytes + content-type. + /// Resolves the lossless source artifact and its real MIME as a non-buffering disk stream — the existing + /// read path. Shared by the explicit-lossless branch and the Opus fallback so both produce identical + /// bytes + content-type. The returned stream is seekable, so the delivery layer's Range→206 still works. /// private async Task ResolveLosslessAsync(string entryKey) { - var source = await _trackContentService.GetAudioBinaryAsync(entryKey); + var source = await _trackContentService.OpenAudioMediaStreamAsync(entryKey); if (source is null) return null; - return new ResolvedAudio(source, MimeTypeExtensions.GetMimeType(source.Extension), AudioFormat.Lossless); + return new ResolvedAudio( + source.Stream, MimeTypeExtensions.GetMimeType(source.Extension), AudioFormat.Lossless); } /// @@ -98,13 +112,18 @@ public sealed class TrackFormatResolver /// public async Task HasOpusAsync(string entryKey) { - var audio = await _fileDatabase.LoadResourceAsync( - VaultConstants.TrackOpus, OpusTranscodeService.OpusAudioKey(entryKey)); - if (audio is null) + // Index-only existence — never read a file body. The opus-status admin endpoint calls this in a loop + // over the entire catalogue, so a body load here would stream the whole library's audio sequentially. + // HasIndexEntry is a pure in-memory index lookup (no disk read, no allocation per track). + var opusVault = _fileDatabase.GetVault(VaultConstants.TrackOpus); + if (opusVault is null) return false; - var sidecar = await _fileDatabase.LoadResourceAsync( - VaultConstants.TrackOpus, OpusTranscodeService.OpusSidecarKey(entryKey)); - return sidecar is not null; + if (!await opusVault.HasIndexEntry(OpusTranscodeService.OpusAudioKey(entryKey))) + return false; + + // Both halves required: audio without the seek/setup sidecar is unseekable, so a half-derived track + // counts as not-having-Opus (the same completeness rule the Backfill-Opus pass enqueues against). + return await opusVault.HasIndexEntry(OpusTranscodeService.OpusSidecarKey(entryKey)); } } diff --git a/DeepDrftTests/TrackFormatDeliveryTests.cs b/DeepDrftTests/TrackFormatDeliveryTests.cs index eba434d..7629b1d 100644 --- a/DeepDrftTests/TrackFormatDeliveryTests.cs +++ b/DeepDrftTests/TrackFormatDeliveryTests.cs @@ -26,10 +26,10 @@ namespace DeepDrftTests; /// required to assert the delivery contract. /// /// The Range→206 contract is asserted at the load-bearing seam: ASP.NET performs the actual byte-slicing -/// for any whose .EnableRangeProcessing is true. The lossless -/// path proves this via the disk-stream ; the resolved Opus path via the -/// in-memory — both must report range processing enabled, the explicit fix -/// the 18.2 reviewer flagged for the byte[] path. +/// for any whose .EnableRangeProcessing is true over a +/// seekable stream. Both the lossless path AND the resolved Opus path now serve a disk-backed +/// (read-path streaming — no whole-file byte[]); both must report range +/// processing enabled, and the FileStream is seekable, so an incoming Range yields a 206 slice. /// [TestFixture] public class TrackFormatDeliveryTests @@ -59,13 +59,14 @@ public class TrackFormatDeliveryTests var result = await controller.GetTrack(entryKey, format: "opus"); - var file = result as FileContentResult; - Assert.That(file, Is.Not.Null, "Opus delivery serves an in-memory byte[] (FileContentResult)"); + var file = result as FileStreamResult; + Assert.That(file, Is.Not.Null, "Opus delivery streams from disk (FileStreamResult), not a byte[]"); + var bytes = await ReadAllAsync(file!.FileStream); Assert.Multiple(() => { - Assert.That(file!.ContentType, Is.EqualTo("audio/ogg"), "Opus bytes must carry the audio/ogg content-type"); - Assert.That(file.FileContents, Is.EqualTo(OpusBytes), "The served bytes must be the Opus artifact, not the source"); - Assert.That(file.EnableRangeProcessing, Is.True, "Range processing must be enabled on the resolved Opus byte[] path"); + Assert.That(file.ContentType, Is.EqualTo("audio/ogg"), "Opus bytes must carry the audio/ogg content-type"); + Assert.That(bytes, Is.EqualTo(OpusBytes), "The served bytes must be the Opus artifact, not the source"); + Assert.That(file.EnableRangeProcessing, Is.True, "Range processing must be enabled on the resolved Opus stream path"); }); } @@ -80,12 +81,13 @@ public class TrackFormatDeliveryTests var result = await controller.GetTrack(entryKey, format: "opus"); - var file = result as FileContentResult; - Assert.That(file, Is.Not.Null, "The fallback still serves resolved bytes via the byte[] path"); + var file = result as FileStreamResult; + Assert.That(file, Is.Not.Null, "The fallback still streams resolved bytes from disk (FileStreamResult)"); + var bytes = await ReadAllAsync(file!.FileStream); Assert.Multiple(() => { - Assert.That(file!.ContentType, Is.EqualTo("audio/wav"), "Fallback content-type must be the lossless source's MIME"); - Assert.That(file.FileContents, Is.EqualTo(_sourceWav), "Fallback must serve the lossless source bytes"); + Assert.That(file.ContentType, Is.EqualTo("audio/wav"), "Fallback content-type must be the lossless source's MIME"); + Assert.That(bytes, Is.EqualTo(_sourceWav), "Fallback must serve the lossless source bytes"); Assert.That(file.EnableRangeProcessing, Is.True, "Range processing stays enabled on the fallback path too"); }); } @@ -167,6 +169,18 @@ public class TrackFormatDeliveryTests private byte[] _sourceWav = []; + // Drains a FileStreamResult's disk-backed stream to a byte[] and disposes it (read-path streaming serves + // an open FileStream, not a buffered byte[]). Disposing also releases the handle before temp-dir teardown. + private static async Task ReadAllAsync(Stream stream) + { + await using (stream) + { + using var ms = new MemoryStream(); + await stream.CopyToAsync(ms); + return ms.ToArray(); + } + } + private async Task FreshDbAsync() { var db = await FileDb.FromAsync(_testDir); diff --git a/DeepDrftTests/TrackFormatResolverTests.cs b/DeepDrftTests/TrackFormatResolverTests.cs index 8aa08f2..d93bf12 100644 --- a/DeepDrftTests/TrackFormatResolverTests.cs +++ b/DeepDrftTests/TrackFormatResolverTests.cs @@ -39,8 +39,8 @@ public class TrackFormatResolverTests private static TrackFormatResolver CreateResolver(FileDb fileDatabase) { - // The resolver only calls GetAudioBinaryAsync (a vault read), which never touches the router — - // but TrackContentService requires one, so supply a real router with real processors. + // The resolver only opens vault streams / index checks, which never touch the router — but + // TrackContentService requires one, so supply a real router with real processors. var router = new AudioProcessorRouter( new AudioProcessor(), new Mp3AudioProcessor(), new FlacAudioProcessor()); var contentService = new TrackContentService(fileDatabase, router); @@ -48,6 +48,18 @@ public class TrackFormatResolverTests fileDatabase, contentService, NullLogger.Instance); } + // Drains a ResolvedAudio's disk-backed stream to a byte[] and disposes it, so the assertions compare the + // bytes actually served (read-path streaming returns an open Stream, not a buffered AudioBinary). + private static async Task ReadAllAsync(Stream stream) + { + await using (stream) + { + using var ms = new MemoryStream(); + await stream.CopyToAsync(ms); + return ms.ToArray(); + } + } + // Seeds a source artifact in the tracks vault with the given extension, mirroring how the upload path // stores the original bytes (WAV/MP3/FLAC). Returns the bytes for downstream identity assertions. private static async Task SeedSourceAsync(FileDb db, string entryKey, string extension) @@ -63,6 +75,14 @@ public class TrackFormatResolverTests // Seeds the Opus audio + sidecar in the track-opus vault exactly as OpusTranscodeService does: // audio under OpusAudioKey (the bare EntryKey) with the .opus extension, sidecar under OpusSidecarKey. private static async Task<(byte[] opus, byte[] sidecar)> SeedOpusAsync(FileDb db, string entryKey) + { + var opusBytes = await SeedOpusAudioOnlyAsync(db, entryKey); + var sidecarBytes = await SeedOpusSidecarOnlyAsync(db, entryKey); + return (opusBytes, sidecarBytes); + } + + // Seeds only the Opus audio half (no sidecar) — a half-derived track, used to prove HasOpusAsync rejects it. + private static async Task SeedOpusAudioOnlyAsync(FileDb db, string entryKey) { await db.CreateVaultAsync(VaultConstants.TrackOpus, MediaVaultType.Audio); @@ -72,6 +92,13 @@ public class TrackFormatResolverTests var audioOk = await db.RegisterResourceAsync( VaultConstants.TrackOpus, OpusTranscodeService.OpusAudioKey(entryKey), opusAudio); Assert.That(audioOk, Is.True, "opus audio seed must succeed"); + return opusBytes; + } + + // Seeds only the sidecar half (no Opus audio) — the other half-derived case HasOpusAsync must reject. + private static async Task SeedOpusSidecarOnlyAsync(FileDb db, string entryKey) + { + await db.CreateVaultAsync(VaultConstants.TrackOpus, MediaVaultType.Audio); var sidecarBytes = new byte[] { 7, 7, 7, 7 }; var sidecar = new MediaBinary(new MediaBinaryParams( @@ -79,8 +106,7 @@ public class TrackFormatResolverTests var sidecarOk = await db.RegisterResourceAsync( VaultConstants.TrackOpus, OpusTranscodeService.OpusSidecarKey(entryKey), sidecar); Assert.That(sidecarOk, Is.True, "sidecar seed must succeed"); - - return (opusBytes, sidecarBytes); + return sidecarBytes; } [Test] @@ -98,8 +124,8 @@ public class TrackFormatResolverTests Assert.That(resolved, Is.Not.Null); Assert.That(resolved!.ResolvedFormat, Is.EqualTo(AudioFormat.Lossless)); Assert.That(resolved.ContentType, Is.EqualTo("audio/flac")); - Assert.That(resolved.Audio.Buffer, Is.EqualTo(bytes)); Assert.That(resolved.DidFallBack(AudioFormat.Lossless), Is.False); + Assert.That(await ReadAllAsync(resolved.Stream), Is.EqualTo(bytes)); } [Test] @@ -116,8 +142,8 @@ public class TrackFormatResolverTests Assert.That(resolved, Is.Not.Null); Assert.That(resolved!.ResolvedFormat, Is.EqualTo(AudioFormat.Opus)); Assert.That(resolved.ContentType, Is.EqualTo("audio/ogg")); - Assert.That(resolved.Audio.Buffer, Is.EqualTo(opusBytes)); Assert.That(resolved.DidFallBack(AudioFormat.Opus), Is.False); + Assert.That(await ReadAllAsync(resolved.Stream), Is.EqualTo(opusBytes)); } [Test] @@ -136,8 +162,8 @@ public class TrackFormatResolverTests "the resolved format must reflect what was actually returned"); Assert.That(resolved.ContentType, Is.EqualTo("audio/wav"), "a fallback returns the lossless content-type so the decoder picks the right decoder"); - Assert.That(resolved.Audio.Buffer, Is.EqualTo(bytes)); Assert.That(resolved.DidFallBack(AudioFormat.Opus), Is.True); + Assert.That(await ReadAllAsync(resolved.Stream), Is.EqualTo(bytes)); } [Test] @@ -177,4 +203,92 @@ public class TrackFormatResolverTests Assert.That(bytes, Is.Null); } + + // --- HasOpusAsync completeness matrix (both halves required) --- + + [Test] + public async Task HasOpusAsync_WhenAudioAndSidecarPresent_ReturnsTrue() + { + var db = (await FileDb.FromAsync(_testDir))!; + const string entryKey = "complete-opus"; + await SeedOpusAsync(db, entryKey); + var resolver = CreateResolver(db); + + Assert.That(await resolver.HasOpusAsync(entryKey), Is.True); + } + + [Test] + public async Task HasOpusAsync_WhenSidecarMissing_ReturnsFalse() + { + var db = (await FileDb.FromAsync(_testDir))!; + const string entryKey = "audio-only-opus"; + await SeedOpusAudioOnlyAsync(db, entryKey); + var resolver = CreateResolver(db); + + Assert.That(await resolver.HasOpusAsync(entryKey), Is.False, + "audio without the seek/setup sidecar is unseekable — counts as not-having-Opus"); + } + + [Test] + public async Task HasOpusAsync_WhenAudioMissing_ReturnsFalse() + { + var db = (await FileDb.FromAsync(_testDir))!; + const string entryKey = "sidecar-only-opus"; + await SeedOpusSidecarOnlyAsync(db, entryKey); + var resolver = CreateResolver(db); + + Assert.That(await resolver.HasOpusAsync(entryKey), Is.False); + } + + [Test] + public async Task HasOpusAsync_WhenNeitherPresent_ReturnsFalse() + { + var db = (await FileDb.FromAsync(_testDir))!; + var resolver = CreateResolver(db); + + // No track-opus vault at all (and no entries) — must miss to false, not throw. + Assert.That(await resolver.HasOpusAsync("ghost"), Is.False); + } + + [Test] + public async Task HasOpusAsync_IsIndexOnly_DoesNotReadFileBodies() + { + var db = (await FileDb.FromAsync(_testDir))!; + const string entryKey = "indexed-but-bodiless"; + await SeedOpusAsync(db, entryKey); + var resolver = CreateResolver(db); + + // Delete the backing files but leave the index entries intact. An index-only existence check still + // reports true; a body-loading implementation would now miss. This is the load-bearing assertion + // that HasOpusAsync never streams a file body (the opus-status loop runs it over the whole catalogue). + var vaultDir = Path.Combine(_testDir, VaultConstants.TrackOpus); + foreach (var file in Directory.EnumerateFiles(vaultDir)) + { + if (Path.GetFileName(file) != "index") + File.Delete(file); + } + + Assert.That(await resolver.HasOpusAsync(entryKey), Is.True, + "HasOpusAsync must be index-only: true even when no file bodies are present"); + } + + [Test] + public async Task ResolveAsync_DisposingResult_DisposesUnderlyingStream() + { + var db = (await FileDb.FromAsync(_testDir))!; + const string entryKey = "dispose-track"; + await SeedSourceAsync(db, entryKey, ".wav"); + var resolver = CreateResolver(db); + + var resolved = await resolver.ResolveAsync(entryKey, AudioFormat.Lossless); + Assert.That(resolved, Is.Not.Null); + + var stream = resolved!.Stream; + Assert.That(stream.CanRead, Is.True, "a freshly resolved handle holds an open stream"); + + await resolved.DisposeAsync(); + + Assert.That(stream.CanRead, Is.False, + "disposing ResolvedAudio must dispose the underlying FileStream so a handle never leaks on the throw path"); + } }