fix(jellycompat): restore CanDownload with a real Download route for Infuse (#123)
dd81a7ef set CanDownload=false to stop Wholphin's screensaver from
404ing on the nonexistent /Items/{id}/Download route — but the flag is
load-bearing for Infuse, which refuses Direct Play (Static=true
streaming) of items it believes it cannot download. With omitempty the
field vanished from the JSON entirely and Infuse playback broke, while
PlaybackInfo-negotiating clients were unaffected.
Resolve the underlying inconsistency instead of trading one client for
the other: implement GET/HEAD /Items/{id}/Download serving the original
file (range support, Content-Disposition, optional mediaSourceId for
multi-version items) under stream-group auth, and restore
CanDownload=true now that the route exists. Fixes Infuse playback and
keeps Wholphin's download callers working.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
co-authored by
Claude Fable 5
parent
c5f21cb10d
commit
0bd4f8cb3b
@@ -80,14 +80,14 @@ Wholphin's Seerr integration (`/api/v1/...` discover/request endpoints) targets
|
||||
|---|---|---|---|
|
||||
| `GET /Items/{id}/ThemeSongs` | `ThemeSongPlayer.kt` | ~~404 → feature silently dead.~~ **Resolved 2026-06-09:** now stubbed with an empty `ThemeMediaResult` (including the `OwnerId` field jellyfin-sdk-kotlin requires). | Resolved |
|
||||
| `GET /Audio/{id}/universal` | `ThemeSongPlayer.kt`, `MusicService.kt` | Audio streaming for theme songs and music playback. No music libraries are exposed today, but this is the second half of the theme-song path and the blocker for any future audio support in jellycompat. | Low (today) |
|
||||
| `GET /Items/{id}/Download` | `SlideshowViewModel.kt`, `ScreensaverService.kt` | ~~Inconsistency: `CanDownload=true` while the route 404s.~~ **Resolved 2026-06-09:** `mapping.go` now sets `CanDownload=false`, so clients no longer attempt downloads. Revisit if a download route is ever implemented. | Resolved |
|
||||
| `GET /Items/{id}/Download` | `SlideshowViewModel.kt`, `ScreensaverService.kt` | ~~Inconsistency: `CanDownload=true` while the route 404s.~~ **Resolved 2026-06-09:** a real `/Items/{id}/Download` route now serves the original file (`streams.go`), and `CanDownload=true` again. The earlier `CanDownload=false` workaround broke Infuse, which requires the flag for Direct Play. | Resolved |
|
||||
| `POST /ClientLog/Document` | `MediaReportService.kt`, `DebugPage.kt` | "Upload logs to server" debug action fails. | Low |
|
||||
| `GET /Audio/{id}/Lyrics` | `NowPlayingViewModel.kt` | Music-only; unreachable until audio libraries exist. | Low |
|
||||
|
||||
## Recommendations (priority order)
|
||||
|
||||
1. **Merge the repeated-`Fields` fix** (PR #110). `parseItemsQuery` still reads `q.Get("Fields")` (`internal/jellycompat/query.go`), which truncates jellyfin-sdk-kotlin's repeated query params and breaks Wholphin episode auto-advance. This is the only *covered* endpoint with a known correctness bug for this client. Follow up by sweeping the remaining single-value reads (`Ids`, `GenreIds`, `PersonIds`, `Filters`, `SortBy`, `SortOrder`) to `q.Values`, matching how `IncludeItemTypes`/`MediaTypes`/`ImageTypes` are already handled.
|
||||
2. ~~Resolve the `CanDownload` inconsistency~~ — **done 2026-06-09**: `CanDownload=false` until a download route exists.
|
||||
2. ~~Resolve the `CanDownload` inconsistency~~ — **done 2026-06-09**: real `/Items/{id}/Download` route added; `CanDownload` stays `true` (Infuse requires it for Direct Play).
|
||||
3. ~~Stub `GET /Items/{id}/ThemeSongs`~~ — **done 2026-06-09**: empty `ThemeMediaResult` stub registered.
|
||||
4. **Websocket server-push** (remote control: server-initiated pause/stop, display messages) — tracked in issue #122.
|
||||
5. **Optional, later:** `POST /ClientLog/Document` accept-and-discard stub and real `/Studios` data. Playlists, lyrics, and universal audio only matter once silo exposes those content types through jellycompat.
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
package jellycompat
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"github.com/Silo-Server/silo-server/internal/catalog"
|
||||
"github.com/Silo-Server/silo-server/internal/models"
|
||||
)
|
||||
|
||||
// TestHandleDownload_ServesOriginalFile verifies /Items/{id}/Download streams
|
||||
// the original media file. The route backs CanDownload=true, which Infuse
|
||||
// requires before it will Direct Play an item.
|
||||
func TestHandleDownload_ServesOriginalFile(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
filePath := filepath.Join(dir, "movie.mkv")
|
||||
content := []byte("fake media bytes")
|
||||
if err := os.WriteFile(filePath, content, 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
codec := NewResourceIDCodec()
|
||||
contentID := "movie-1"
|
||||
detail := &upstreamItemDetail{
|
||||
ContentID: contentID,
|
||||
Type: "movie",
|
||||
Versions: []catalog.FileVersion{{
|
||||
FileID: 42,
|
||||
FilePath: filePath,
|
||||
Container: "mkv",
|
||||
Duration: 3600,
|
||||
AddedAt: time.Now(),
|
||||
}},
|
||||
}
|
||||
handler := &PlaybackHandler{
|
||||
codec: codec,
|
||||
content: &stubContentService{detail: detail},
|
||||
fileResolver: testCompatFileResolver{file: &models.MediaFile{ID: 42, FilePath: filePath}},
|
||||
}
|
||||
|
||||
encodedID := codec.EncodeStringID(EncodedIDItem, contentID)
|
||||
req := httptest.NewRequest("GET", "/Items/"+encodedID+"/Download", nil)
|
||||
routeCtx := chi.NewRouteContext()
|
||||
routeCtx.URLParams.Add("id", encodedID)
|
||||
ctx := context.WithValue(req.Context(), chi.RouteCtxKey, routeCtx)
|
||||
ctx = context.WithValue(ctx, compatSessionKey, &Session{StreamAppUserID: 1, ProfileID: "profile-1"})
|
||||
req = req.WithContext(ctx)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.HandleDownload(rec, req)
|
||||
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("expected status 200; got %d, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if got := rec.Body.String(); got != string(content) {
|
||||
t.Errorf("expected file content %q; got %q", content, got)
|
||||
}
|
||||
if cd := rec.Header().Get("Content-Disposition"); cd == "" {
|
||||
t.Error("expected Content-Disposition header on download response")
|
||||
}
|
||||
}
|
||||
|
||||
// TestItemDetail_AdvertisesCanDownload guards against regressing the
|
||||
// CanDownload flag: Infuse refuses Direct Play (Static=true streaming) of
|
||||
// items it believes it cannot download, so playable items must advertise it.
|
||||
func TestItemDetail_AdvertisesCanDownload(t *testing.T) {
|
||||
m := newMapper(NewResourceIDCodec(), nil)
|
||||
detail := upstreamItemDetail{
|
||||
ContentID: "movie-1",
|
||||
Type: "movie",
|
||||
Versions: []catalog.FileVersion{{
|
||||
FileID: 42,
|
||||
Container: "mkv",
|
||||
Duration: 3600,
|
||||
AddedAt: time.Now(),
|
||||
}},
|
||||
}
|
||||
dto := m.itemFromDetailWithFields(detail, false, nil, nil)
|
||||
if !dto.CanDownload {
|
||||
t.Error("playable item detail must advertise CanDownload=true; Infuse requires it for Direct Play")
|
||||
}
|
||||
}
|
||||
@@ -382,9 +382,10 @@ func (m *mapper) itemFromDetailWithFields(item upstreamItemDetail, isFavorite bo
|
||||
dto.RunTimeTicks = secondsToTicks(float64(firstVersion.Duration))
|
||||
}
|
||||
dto.DateCreated = formatCompatTime(firstVersion.AddedAt)
|
||||
// No /Items/{id}/Download route exists; advertising download support
|
||||
// sends clients (e.g. Wholphin's screensaver) into 404s.
|
||||
dto.CanDownload = false
|
||||
// CanDownload is load-bearing for Infuse: it refuses Direct Play
|
||||
// (Static=true streaming) of items it believes it cannot download.
|
||||
// The flag is backed by the /Items/{id}/Download route (streams.go).
|
||||
dto.CanDownload = true
|
||||
dto.HasSubtitles = versionsHaveSubtitles(item.Versions)
|
||||
dto.SupportsSync = false
|
||||
dto.Container = strings.ToLower(firstVersion.Container)
|
||||
|
||||
@@ -30,6 +30,7 @@ var compatPathSegments = map[string]string{
|
||||
"themesongs": "ThemeSongs",
|
||||
"specialfeatures": "SpecialFeatures",
|
||||
"intros": "Intros",
|
||||
"download": "Download",
|
||||
"images": "Images",
|
||||
"primary": "Primary",
|
||||
"backdrop": "Backdrop",
|
||||
|
||||
@@ -218,6 +218,8 @@ func NewRouter(deps Dependencies) chi.Router {
|
||||
// (e.g. libmpv) that don't forward auth headers or query parameters.
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(PlaybackSessionAuth(deps.SessionStore, deps.PlaybackStore, adminAPIKeyAuth))
|
||||
r.Method(http.MethodHead, "/Items/{id}/Download", http.HandlerFunc(playbackHandler.HandleDownload))
|
||||
r.Get("/Items/{id}/Download", playbackHandler.HandleDownload)
|
||||
r.Method(http.MethodHead, "/Videos/{id}/stream", http.HandlerFunc(playbackHandler.HandleVideoStream))
|
||||
r.Get("/Videos/{id}/stream", playbackHandler.HandleVideoStream)
|
||||
r.Method(http.MethodHead, "/Videos/{id}/stream.{container}", http.HandlerFunc(playbackHandler.HandleVideoStream))
|
||||
|
||||
@@ -113,6 +113,55 @@ func (h *PlaybackHandler) HandleVideoStream(w http.ResponseWriter, r *http.Reque
|
||||
}
|
||||
}
|
||||
|
||||
// HandleDownload serves the original media file for /Items/{id}/Download.
|
||||
// This route backs the CanDownload flag set in mapping.go. CanDownload is
|
||||
// load-bearing for Infuse: it refuses Direct Play (Static=true streaming)
|
||||
// for items it believes it cannot download, so the flag must stay true and
|
||||
// this route must exist.
|
||||
func (h *PlaybackHandler) HandleDownload(w http.ResponseWriter, r *http.Request) {
|
||||
session := SessionFromContext(r.Context())
|
||||
if session == nil {
|
||||
writeError(w, http.StatusUnauthorized, "Unauthorized", "Missing authentication token")
|
||||
return
|
||||
}
|
||||
|
||||
contentID, err := decodeContentID(h.codec, chiURLParam(r, "id"))
|
||||
if err != nil {
|
||||
writeError(w, http.StatusNotFound, "NotFound", "Item not found")
|
||||
return
|
||||
}
|
||||
detail, err := h.content.GetItemDetail(r.Context(), session, contentID, nil)
|
||||
if err != nil || detail == nil || len(detail.Versions) == 0 {
|
||||
writeError(w, http.StatusNotFound, "NotFound", "Item not found")
|
||||
return
|
||||
}
|
||||
|
||||
version := detail.Versions[0]
|
||||
if mediaSourceID := firstNonEmpty(r.URL.Query().Get("mediaSourceId"), r.URL.Query().Get("MediaSourceId")); mediaSourceID != "" {
|
||||
if fileID, decodeErr := h.codec.DecodeIntID(EncodedIDMediaSource, mediaSourceID); decodeErr == nil {
|
||||
for _, v := range detail.Versions {
|
||||
if int64(v.FileID) == fileID {
|
||||
version = v
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if h.fileResolver == nil {
|
||||
writeError(w, http.StatusInternalServerError, "ServerError", "File resolver not available")
|
||||
return
|
||||
}
|
||||
file, err := h.fileResolver.GetByID(r.Context(), version.FileID)
|
||||
if err != nil {
|
||||
writeError(w, http.StatusNotFound, "NotFound", "Media file not found")
|
||||
return
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Disposition", "attachment; filename*=UTF-8''"+url.PathEscape(filepath.Base(file.FilePath)))
|
||||
_ = playback.ServeDirectPlay(w, r, file.FilePath)
|
||||
}
|
||||
|
||||
// HandleMasterManifest serves the compat-owned HLS manifest route.
|
||||
// It returns a full-duration VOD manifest so clients can seek to any position.
|
||||
// Segments that haven't been transcoded yet are served on-demand by the segment handler.
|
||||
|
||||
Reference in New Issue
Block a user