Compare commits
3 commits
b31377c1e9
...
310dcd422e
| Author | SHA1 | Date | |
|---|---|---|---|
| 310dcd422e | |||
| 6bec09ae20 | |||
| 5b2b832be7 |
4 changed files with 71 additions and 2 deletions
|
|
@ -399,6 +399,10 @@ Then put it behind your existing reverse proxy like everything else
|
||||||
session" with no working controls — the old device never reacted
|
session" with no working controls — the old device never reacted
|
||||||
to being kicked, and the leave route had no way to tell a stale
|
to being kicked, and the leave route had no way to tell a stale
|
||||||
device apart from the current one — see docs/architecture.md
|
device apart from the current one — see docs/architecture.md
|
||||||
|
- [x] Fixed a track started directly (not via its album/artist) never
|
||||||
|
falling back to its album's artwork — see docs/architecture.md
|
||||||
|
(probably not the whole "images are hard to find" story — flagged
|
||||||
|
there)
|
||||||
- [ ] Try a full party end-to-end with two real accounts (still the main gap)
|
- [ ] Try a full party end-to-end with two real accounts (still the main gap)
|
||||||
- [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is
|
- [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is
|
||||||
confirmed here — no real device/session)
|
confirmed here — no real device/session)
|
||||||
|
|
|
||||||
|
|
@ -4478,6 +4478,45 @@ harness - hls.js/a full player DOM would need mocking for no more
|
||||||
signal than the MusicBar case already gives, given both share the same
|
signal than the MusicBar case already gives, given both share the same
|
||||||
logic shape end to end).
|
logic shape end to end).
|
||||||
|
|
||||||
|
## A track started directly never fell back to its album's artwork
|
||||||
|
|
||||||
|
Reported directly: the app struggles to find artist/album/track images.
|
||||||
|
Traced one concrete, confirmed cause: `imageUrlFor`'s own
|
||||||
|
`Audio -> AlbumId` fallback (added earlier specifically because a track
|
||||||
|
essentially never has its own Primary image, only its album usually
|
||||||
|
does) was silently never triggering for a track played directly - not
|
||||||
|
via its album or artist - because the raw item that decision runs
|
||||||
|
against, `CatalogService.getRawItem`, never requested `AlbumId` from
|
||||||
|
Emby in the first place. Emby's default field set for a single-item GET
|
||||||
|
omits it entirely, so `item.AlbumId` read `undefined` there regardless
|
||||||
|
of whether the track's actual album genuinely had one - `imageUrlFor`
|
||||||
|
fell through to the track's own id every time, for every party started
|
||||||
|
this way.
|
||||||
|
|
||||||
|
Every OTHER path that resolves a track's image already asks for it
|
||||||
|
correctly - `getAlbumTracks`/`getPlaylistItemsRaw` both already include
|
||||||
|
`AlbumId` in their own `Fields`, which is why launching a whole album/
|
||||||
|
artist/playlist, or a queue built from one, was never affected. Fixed
|
||||||
|
the one gap instead of guessing at a broader rewrite: `getRawItem` now
|
||||||
|
requests `Fields=AlbumId` too. Harmless for every non-Audio item it also
|
||||||
|
fetches (Movie/Series/Episode/...) - Emby just omits a field that
|
||||||
|
genuinely doesn't apply to them.
|
||||||
|
|
||||||
|
Not the whole story, most likely - a real Emby library can also
|
||||||
|
legitimately have no Primary image at all for a given artist or album
|
||||||
|
(nothing for this app to fall back to, no bug to fix), and this fix
|
||||||
|
only ever covers the one specific path (a track started directly) that
|
||||||
|
was provably missing a field it needed. Flagged directly rather than
|
||||||
|
overclaiming "images are fixed" - worth another, more specific report
|
||||||
|
(which item exactly, from where) if artist/album art still struggles
|
||||||
|
after this.
|
||||||
|
|
||||||
|
Verified: full `typecheck`/`test`/`build` (server only - `CatalogService.ts`
|
||||||
|
is server-side). New test confirms `getRawItem`'s actual outgoing
|
||||||
|
request includes `Fields=AlbumId` (a mocked `EmbyClient.fetch` capturing
|
||||||
|
the real path/query it was called with, not just asserting on the
|
||||||
|
already-separately-tested `imageUrlFor` logic downstream of it).
|
||||||
|
|
||||||
## Known limitations
|
## Known limitations
|
||||||
|
|
||||||
- Playback always goes through `hls.js` against whatever Emby's
|
- Playback always goes through `hls.js` against whatever Emby's
|
||||||
|
|
|
||||||
|
|
@ -125,9 +125,23 @@ class CatalogServiceClass {
|
||||||
* mediaType from the clicked item's real Emby Type (see index.ts).
|
* mediaType from the clicked item's real Emby Type (see index.ts).
|
||||||
* Unlike every other method here, this preserves Emby's own status
|
* Unlike every other method here, this preserves Emby's own status
|
||||||
* (typically a real 404) rather than a blanket 502 - "titre introuvable"
|
* (typically a real 404) rather than a blanket 502 - "titre introuvable"
|
||||||
* is a not-found, not an upstream failure. */
|
* is a not-found, not an upstream failure.
|
||||||
|
*
|
||||||
|
* `Fields=AlbumId` - without it, Emby's default field set for a single
|
||||||
|
* item omits it, so a track started directly (not via its album/
|
||||||
|
* artist - the one path that reaches imageUrlFor through *this*
|
||||||
|
* method, in index.ts's POST /api/parties) always missed imageUrlFor's
|
||||||
|
* own Audio -> AlbumId fallback and fell through to the track's own
|
||||||
|
* id instead - which, confirmed directly against a real server (see
|
||||||
|
* imageUrlFor's own comment), essentially never has a Primary image
|
||||||
|
* of its own. Reported directly as "the app really struggles to find
|
||||||
|
* artist/album/track images" - this is the one concrete, fixable
|
||||||
|
* piece of that: a party started this way now gets the same album
|
||||||
|
* art every other path already resolves correctly. Harmless for
|
||||||
|
* every non-Audio item this also fetches (Movie/Series/Episode/...) -
|
||||||
|
* they have no AlbumId to begin with, so Emby just omits the field. */
|
||||||
async getRawItem(session: EmbySession, itemId: string): Promise<EmbyItem> {
|
async getRawItem(session: EmbySession, itemId: string): Promise<EmbyItem> {
|
||||||
return this.fetchJson<EmbyItem>(`/Users/${session.userId}/Items/${itemId}`, session, 'Titre introuvable.', { preserveEmbyStatus: true });
|
return this.fetchJson<EmbyItem>(`/Users/${session.userId}/Items/${itemId}?Fields=AlbumId`, session, 'Titre introuvable.', { preserveEmbyStatus: true });
|
||||||
}
|
}
|
||||||
|
|
||||||
async getItem(session: EmbySession, itemId: string): Promise<ItemDto> {
|
async getItem(session: EmbySession, itemId: string): Promise<ItemDto> {
|
||||||
|
|
|
||||||
|
|
@ -97,6 +97,18 @@ describe('CatalogService', () => {
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('getRawItem requests Fields=AlbumId - without it, a track started directly (not via its album) always missed imageUrlFor\'s own fallback', async (t) => {
|
||||||
|
let requestedPath = '';
|
||||||
|
t.mock.method(EmbyClient, 'fetch', async (pathAndQuery: string) => {
|
||||||
|
requestedPath = pathAndQuery;
|
||||||
|
return jsonResponse({ Id: 't1', Name: 'Track', Type: 'Audio', AlbumId: 'al1' });
|
||||||
|
});
|
||||||
|
|
||||||
|
await CatalogService.getRawItem(session, 't1');
|
||||||
|
|
||||||
|
assert.ok(requestedPath.includes('Fields=AlbumId'), requestedPath);
|
||||||
|
});
|
||||||
|
|
||||||
it('getPlaybackOptions flags an image/bitmap subtitle (PGS) as not text-based, real text subtitles as fine', async (t) => {
|
it('getPlaybackOptions flags an image/bitmap subtitle (PGS) as not text-based, real text subtitles as fine', async (t) => {
|
||||||
t.mock.method(EmbyClient, 'fetch', async () => jsonResponse({
|
t.mock.method(EmbyClient, 'fetch', async () => jsonResponse({
|
||||||
MediaSources: [{
|
MediaSources: [{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue