Skip to content

Seekable streams (Safari/iOS fix for video) #580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3rob3 wants to merge 9 commits into from
+85 −27
Closed

Seekable streams (Safari/iOS fix for video) #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aa02e01
Fix video playback on Safari/iOS by ensuring seekable streams. Implem…
Feb 2, 2026
4ed4fb3
coderabbit suggestion
Feb 2, 2026
1b7e7c6
Change from memorystream to temp file
Feb 9, 2026
a804690
temp-then-rename pattern
Feb 9, 2026
6a24e12
Proper range header handling
Feb 19, 2026
8624060
disposal
Feb 19, 2026
99a17c7
fix webhook spamming
Feb 19, 2026
8198657
indent
Feb 19, 2026
8a9ad8c
dispose response
Feb 19, 2026
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
2 changes: 1 addition & 1 deletion ImmichFrame.Core/Interfaces/IImmichFrameLogic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public interface IImmichFrameLogic
public Task<IEnumerable<AssetResponseDto>> GetAssets();
public Task<AssetResponseDto> GetAssetInfoById(Guid assetId);
public Task<IEnumerable<AlbumResponseDto>> GetAlbumInfoById(Guid assetId);
public Task<(string fileName, string ContentType, Stream fileStream)> GetAsset(Guid id, AssetTypeEnum? assetType = null);
public Task<(string fileName, string ContentType, Stream fileStream, string? contentRange, bool isPartial, IDisposable? dispose, string? contentLength)> GetAsset(Guid id, AssetTypeEnum? assetType = null, string? rangeHeader = null);
public Task<long> GetTotalAssets();
public Task SendWebhookNotification(IWebhookNotification notification);
}
Expand Down
4 changes: 2 additions & 2 deletions ImmichFrame.Core/Logic/MultiImmichFrameLogicDelegate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public Task<IEnumerable<AlbumResponseDto>> GetAlbumInfoById(Guid assetId)
=> _accountSelectionStrategy.ForAsset(assetId, logic => logic.GetAlbumInfoById(assetId));


public Task<(string fileName, string ContentType, Stream fileStream)> GetAsset(Guid assetId, AssetTypeEnum? assetType = null)
=> _accountSelectionStrategy.ForAsset(assetId, logic => logic.GetAsset(assetId, assetType));
public Task<(string fileName, string ContentType, Stream fileStream, string? contentRange, bool isPartial, IDisposable? dispose, string? contentLength)> GetAsset(Guid assetId, AssetTypeEnum? assetType = null, string? rangeHeader = null)
=> _accountSelectionStrategy.ForAsset(assetId, logic => logic.GetAsset(assetId, assetType, rangeHeader));

public async Task<long> GetTotalAssets()
{
Expand Down
64 changes: 46 additions & 18 deletions ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using ImmichFrame.Core.Helpers;
using ImmichFrame.Core.Interfaces;
using ImmichFrame.Core.Logic.Pool;
using System.Net.Http.Headers;

namespace ImmichFrame.Core.Logic;

Expand All @@ -12,6 +13,7 @@ public class PooledImmichFrameLogic : IAccountImmichFrameLogic
private readonly IApiCache _apiCache;
private readonly IAssetPool _pool;
private readonly ImmichApi _immichApi;
private readonly HttpClient _httpClient;
private readonly string _downloadLocation = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "ImageCache");

public PooledImmichFrameLogic(IAccountSettings accountSettings, IGeneralSettings generalSettings, IHttpClientFactory httpClientFactory)
Expand All @@ -22,6 +24,7 @@ public PooledImmichFrameLogic(IAccountSettings accountSettings, IGeneralSettings
AccountSettings = accountSettings;

httpClient.UseApiKey(accountSettings.ApiKey);
_httpClient = httpClient;
_immichApi = new ImmichApi(accountSettings.ImmichServerUrl, httpClient);

_apiCache = new ApiCache(RefreshInterval(generalSettings.RefreshAlbumPeopleInterval));
Expand Down Expand Up @@ -80,7 +83,7 @@ public Task<IEnumerable<AssetResponseDto>> GetAssets()

public Task<long> GetTotalAssets() => _pool.GetAssetCount();

public async Task<(string fileName, string ContentType, Stream fileStream)> GetAsset(Guid id, AssetTypeEnum? assetType = null)
public async Task<(string fileName, string ContentType, Stream fileStream, string? contentRange, bool isPartial, IDisposable? dispose, string? contentLength)> GetAsset(Guid id, AssetTypeEnum? assetType = null, string? rangeHeader = null)
{
if (!assetType.HasValue)
{
Expand All @@ -92,17 +95,17 @@ public Task<IEnumerable<AssetResponseDto>> GetAssets()

if (assetType == AssetTypeEnum.IMAGE)
{
return await GetImageAsset(id);
var (fileName, contentType, fileStream) = await GetImageAsset(id);
return (fileName, contentType, fileStream, null, false, null, null);
}

if (assetType == AssetTypeEnum.VIDEO)
{
return await GetVideoAsset(id);
return await GetVideoAsset(id, rangeHeader);
}

throw new AssetNotFoundException($"Asset {id} is not a supported media type ({assetType}).");
}

private async Task<(string fileName, string ContentType, Stream fileStream)> GetImageAsset(Guid id)
{
if (_generalSettings.DownloadImages)
Expand Down Expand Up @@ -160,29 +163,54 @@ public Task<IEnumerable<AssetResponseDto>> GetAssets()
return (fileName, contentType, data.Stream);
}

private async Task<(string fileName, string ContentType, Stream fileStream)> GetVideoAsset(Guid id)
private async Task<FileResponse> PlayVideoWithRange(Guid id, string rangeHeader, CancellationToken cancellationToken = default)
{
var videoResponse = await _immichApi.PlayAssetVideoAsync(id, string.Empty);
var url = $"{_immichApi.BaseUrl.TrimEnd('/')}/assets/{id}/video/playback";

if (videoResponse == null)
throw new AssetNotFoundException($"Video asset {id} was not found!");
using var request = new HttpRequestMessage(HttpMethod.Get, url);
request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/octet-stream"));
request.Headers.TryAddWithoutValidation("Range", rangeHeader);

var contentType = "";
if (videoResponse.Headers.ContainsKey("Content-Type"))
{
contentType = videoResponse.Headers["Content-Type"].FirstOrDefault() ?? "";
}
var response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken);

var headers = response.Headers.ToDictionary(h => h.Key, h => h.Value);
if (response.Content?.Headers != null)
foreach (var item in response.Content.Headers)
headers[item.Key] = item.Value;

if (string.IsNullOrWhiteSpace(contentType))
var status = (int)response.StatusCode;
if (status == 200 || status == 206)
{
contentType = "video/mp4";
var stream = response.Content == null ? Stream.Null : await response.Content.ReadAsStreamAsync(cancellationToken);
return new FileResponse(status, headers, stream, null, response);
}

var fileName = $"{id}.mp4";

return (fileName, contentType, videoResponse.Stream);
var error = response.Content == null ? null : await response.Content.ReadAsStringAsync(cancellationToken);
response.Dispose();
throw new ApiException($"Unexpected status code ({status}).", status, error, headers, null);
}

private async Task<(string fileName, string ContentType, Stream fileStream, string? contentRange, bool isPartial, IDisposable? dispose, string? contentLength)> GetVideoAsset(Guid id, string? rangeHeader = null)
{
var videoResponse = string.IsNullOrEmpty(rangeHeader)
? await _immichApi.PlayAssetVideoAsync(id, string.Empty)
: await PlayVideoWithRange(id, rangeHeader);

if (videoResponse == null)
throw new AssetNotFoundException($"Video asset {id} was not found!");

var contentType = videoResponse.Headers.TryGetValue("Content-Type", out var ct)
? ct.FirstOrDefault() ?? "video/mp4"
: "video/mp4";

var contentRange = videoResponse.Headers.TryGetValue("Content-Range", out var cr)
? cr.FirstOrDefault()
: null;

var contentLength = videoResponse.Headers.TryGetValue("Content-Length", out var cl) ? cl.FirstOrDefault() : null;

return ($"{id}.mp4", contentType, videoResponse.Stream, contentRange, videoResponse.StatusCode == 206, videoResponse, contentLength);
}
public Task SendWebhookNotification(IWebhookNotification notification) =>
WebhookHelper.SendWebhookNotification(notification, _generalSettings.Webhook);

Expand Down
42 changes: 36 additions & 6 deletions ImmichFrame.WebApi/Controllers/AssetController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,54 @@ public async Task<List<AlbumResponseDto>> GetAlbumInfo(Guid id, string clientIde
[HttpGet("{id}/Image", Name = "GetImage")]
[Produces("image/jpeg", "image/webp")]
[ProducesResponseType(typeof(FileStreamResult), StatusCodes.Status200OK)]
public async Task<FileResult> GetImage(Guid id, string clientIdentifier = "")
public async Task<IActionResult> GetImage(Guid id, string clientIdentifier = "")
{
return await GetAsset(id, clientIdentifier, AssetTypeEnum.IMAGE);
}

[HttpGet("{id}/Asset", Name = "GetAsset")]
[Produces("image/jpeg", "image/webp", "video/mp4", "video/quicktime")]
[ProducesResponseType(typeof(FileStreamResult), StatusCodes.Status200OK)]
public async Task<FileResult> GetAsset(Guid id, string clientIdentifier = "", AssetTypeEnum? assetType = null)
[ProducesResponseType(typeof(FileStreamResult), StatusCodes.Status206PartialContent)]
Copy link

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

ProducesResponseType for 206 incorrectly declares FileStreamResult.

The 206 branch returns EmptyResult (line 107) after writing directly to Response.Body. Annotating it with typeof(FileStreamResult) generates misleading Swagger/OpenAPI metadata. Use typeof(void) or omit the type argument.

♻️ Proposed fix 
-        [ProducesResponseType(typeof(FileStreamResult), StatusCodes.Status206PartialContent)]
+        [ProducesResponseType(StatusCodes.Status206PartialContent)]
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ImmichFrame.WebApi/Controllers/AssetController.cs` at line 74, The
ProducesResponseType attribute on the AssetController method is incorrect for
206 responses: it currently uses typeof(FileStreamResult) but the 206 branch
writes directly to Response.Body and returns EmptyResult; update the attribute
on the relevant action in AssetController (the method that writes to
Response.Body and returns EmptyResult) to use typeof(void) or remove the type
argument so Swagger/OpenAPI reflects no response body for the 206 branch instead
of FileStreamResult.

public async Task<IActionResult> GetAsset(Guid id, string clientIdentifier = "", AssetTypeEnum? assetType = null)
{
var sanitizedClientIdentifier = clientIdentifier.SanitizeString();
_logger.LogDebug("Asset '{id}' requested by '{sanitizedClientIdentifier}' (type hint: {assetType})", id, sanitizedClientIdentifier, assetType);
var asset = await _logic.GetAsset(id, assetType);

var notification = new AssetRequestedNotification(id, sanitizedClientIdentifier);
_ = _logic.SendWebhookNotification(notification);
var rangeHeader = Request.Headers["Range"].FirstOrDefault();
var asset = await _logic.GetAsset(id, assetType, rangeHeader);

if (string.IsNullOrEmpty(rangeHeader))
{
var notification = new AssetRequestedNotification(id, sanitizedClientIdentifier);
_ = _logic.SendWebhookNotification(notification);
}

Response.Headers["Accept-Ranges"] = "bytes";

if (asset.isPartial && !string.IsNullOrEmpty(asset.contentRange))
{
Response.Headers["Content-Range"] = asset.contentRange;
Response.StatusCode = 206;
Response.ContentType = asset.ContentType;

return File(asset.fileStream, asset.ContentType, asset.fileName, enableRangeProcessing: true);
if (asset.fileStream is { CanSeek: true } && asset.fileStream.Length > 0)
Response.ContentLength = asset.fileStream.Length;
else if (!string.IsNullOrEmpty(asset.contentLength) && long.TryParse(asset.contentLength, out var length))
Response.ContentLength = length;

await using (asset.fileStream)
{
await asset.fileStream.CopyToAsync(Response.Body);
}
asset.dispose?.Dispose();
return new EmptyResult();
}

using (asset.dispose)
{
return File(asset.fileStream, asset.ContentType, asset.fileName, enableRangeProcessing: true);
}
Comment on lines +102 to +113
Copy link

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine PooledImmichFrameLogic.cs to understand what dispose and fileStream are
find . -name "PooledImmichFrameLogic.cs" -type f

Repository: immichFrame/ImmichFrame

Length of output: 117

🏁 Script executed:

# Search for GetImageAsset and GetVideoAsset method implementations
rg -n "GetImageAsset|GetVideoAsset" --type cs -A 15 | head -100

Repository: immichFrame/ImmichFrame

Length of output: 4247

🏁 Script executed:

# Check AssetController.cs to see the full context around lines 102-113
fd "AssetController.cs" --type f

Repository: immichFrame/ImmichFrame

Length of output: 116

🏁 Script executed:

# Read AssetController.cs to see the context around lines 102-113
cat -n ImmichFrame.WebApi/Controllers/AssetController.cs | head -150 | tail -60

Repository: immichFrame/ImmichFrame

Length of output: 3097

🏁 Script executed:

# Get the complete GetImageAsset implementation
rg -n "private async Task.*GetImageAsset" ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs -A 40

Repository: immichFrame/ImmichFrame

Length of output: 1659

🏁 Script executed:

# Get the complete GetVideoAsset implementation
rg -n "private async Task.*GetVideoAsset" ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs -A 80

Repository: immichFrame/ImmichFrame

Length of output: 1528

🏁 Script executed:

# Find the GetAsset method that's called from the controller
rg -n "public.*Task.*GetAsset" ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs -A 25

Repository: immichFrame/ImmichFrame

Length of output: 1777

🏁 Script executed:

# Check if there are other usages of asset in the controller that might affect resource lifecycle
rg -n "asset\." ImmichFrame.WebApi/Controllers/AssetController.cs -B 2 -A 2 | head -80

Repository: immichFrame/ImmichFrame

Length of output: 1515

🏁 Script executed:

# Verify what File() returns and search for documentation or usage patterns
rg -n "File\(" ImmichFrame.WebApi/Controllers/AssetController.cs -B 2 -A 2

Repository: immichFrame/ImmichFrame

Length of output: 269

Two resource-management bugs: asset.dispose leaks on exception (partial path) and is disposed prematurely before FileStreamResult executes (non-partial path).

Partial path (lines 102–107): asset.dispose?.Dispose() is called after the await using block but outside it. If CopyToAsync throws, the await using correctly cleans up asset.fileStream, but asset.dispose (the HttpResponseMessage for video range requests) is never released — leaking the HTTP connection.

Non-partial path (lines 110–113): For video assets, the using block disposes asset.dispose (the HttpResponseMessage) immediately after the return expression is evaluated, before the FileStreamResult is handed to ASP.NET Core. This closes the underlying stream before FileStreamResult.ExecuteResultAsync() executes, causing a read failure. (Image assets are unaffected because asset.dispose is null.) The safe pattern is to register asset.dispose for deferred disposal via HttpContext.Response.RegisterForDispose().

🐛 Proposed fix — partial path 
-                await using (asset.fileStream)
-                {
-                    await asset.fileStream.CopyToAsync(Response.Body);
-                }
-                asset.dispose?.Dispose();
+                try
+                {
+                    await using (asset.fileStream)
+                    {
+                        await asset.fileStream.CopyToAsync(Response.Body);
+                    }
+                }
+                finally
+                {
+                    asset.dispose?.Dispose();
+                }
🐛 Proposed fix — non-partial path 
-            using (asset.dispose)
-            {
-                return File(asset.fileStream, asset.ContentType, asset.fileName, enableRangeProcessing: true);
-            }
+            if (asset.dispose != null)
+                HttpContext.Response.RegisterForDispose(asset.dispose);
+            return File(asset.fileStream, asset.ContentType, asset.fileName, enableRangeProcessing: true);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ImmichFrame.WebApi/Controllers/AssetController.cs` around lines 102 - 113,
The current code disposes asset.dispose incorrectly causing leaks on exceptions
and premature disposal before ASP.NET executes FileStreamResult; fix by
registering the HttpResponseMessage for deferred disposal instead of immediate
Dispose: before copying or returning, call
HttpContext.Response.RegisterForDispose(asset.dispose) when asset.dispose is
non-null so the framework disposes it after the response is complete; remove the
explicit asset.dispose?.Dispose() after the await using and remove the using
(asset.dispose) block around the File(...) return so FileStreamResult can read
the stream during ExecuteResultAsync; keep await using for asset.fileStream but
ensure asset.dispose is registered for disposal up front (reference symbols:
asset.dispose, asset.fileStream, CopyToAsync, File(...)/FileStreamResult,
HttpContext.Response.RegisterForDispose()).

}

[HttpGet("RandomImageAndInfo", Name = "GetRandomImageAndInfo")]
Expand Down
Loading