Merge pull request 'Fix a track started directly never falling back to its album's artwork' (#24) from fix/track-image-albumid-fallback into master
This commit is contained in:
commit
310dcd422e
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
|
||||
to being kicked, and the leave route had no way to tell a stale
|
||||
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 casting to a real Chromecast (only the SDK's own bootstrap is
|
||||
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
|
||||
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
|
||||
|
||||
- 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).
|
||||
* Unlike every other method here, this preserves Emby's own status
|
||||
* (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> {
|
||||
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> {
|
||||
|
|
|
|||
|
|
@ -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) => {
|
||||
t.mock.method(EmbyClient, 'fetch', async () => jsonResponse({
|
||||
MediaSources: [{
|
||||
|
|
|
|||
Loading…
Reference in a new issue