diff --git a/backend/imagemanager.go b/backend/imagemanager.go index f9f0c5f..96da5bf 100644 --- a/backend/imagemanager.go +++ b/backend/imagemanager.go @@ -21,13 +21,13 @@ import ( "github.com/google/uuid" ) -const CachedImageValidTime = 24 * time.Hour - const ( coverArtThumbnailSize = 300 + cachedImageValidTime = 24 * time.Hour fullSizeCoverExpires = 5 * time.Minute - defaultDiskCacheSizeBytes = 50 * 1_048_576 + maxConcurrentServerFetches = 5 + defaultDiskCacheSizeBytes = 50 * 1_048_576 ) // The ImageManager is responsible for retrieving and serving images to the UI layer. @@ -44,8 +44,11 @@ type ImageManager struct { maxOnDiskCacheSizeBytes int64 filesWrittenSinceLastPrune bool + + serverFetchSema chan interface{} } +// NewImageManager returns a new ImageManager. func NewImageManager(ctx context.Context, s *ServerManager, baseCacheDir string) *ImageManager { if err := configdir.MakePath(baseCacheDir); err != nil { log.Println("failed to create album cover cache dir") @@ -60,6 +63,7 @@ func NewImageManager(ctx context.Context, s *ServerManager, baseCacheDir string) DefaultTTL: 1 * time.Minute, }, maxOnDiskCacheSizeBytes: defaultDiskCacheSizeBytes, + serverFetchSema: make(chan interface{}, maxConcurrentServerFetches), } s.OnLogout(func() { i.thumbnailCache.Clear() @@ -73,10 +77,14 @@ func NewImageManager(ctx context.Context, s *ServerManager, baseCacheDir string) return i } +// SetMaxOnDiskCacheSizeBytes sets the maximum size of the on-disc cover thumbnail cache. +// A periodic clean task will delete least recently accessed images to maintain the size limit. func (i *ImageManager) SetMaxOnDiskCacheSizeBytes(size int64) { i.maxOnDiskCacheSizeBytes = size } +// GetCoverThumbnailFromCache returns the cover thumbnail for the given ID if it exists +// in the in-memory cache. Returns quickly, safe to call in UI threads. func (i *ImageManager) GetCoverThumbnailFromCache(coverID string) (image.Image, bool) { img, err := i.thumbnailCache.GetExtendTTL(coverID, i.thumbnailCache.DefaultTTL) if err == nil && img != nil { @@ -85,21 +93,44 @@ func (i *ImageManager) GetCoverThumbnailFromCache(coverID string) (image.Image, return nil, false } +// GetCoverThumbnailAsync asynchronously fetches the cover image for the given ID, +// and invokes the callback on completion. It returns a context.CancelFunc which can be used to +// cancel the fetch. The callback will not be invoked if the fetch is cancelled before completion. +// The cancel func must be invoked to avoid resource leaks. Use GetCoverThumbnail if cancellation is not needed. +func (i *ImageManager) GetCoverThumbnailAsync(coverID string, cb func(image.Image, error)) context.CancelFunc { + ctx, cancel := context.WithCancel(context.Background()) + go func() { + if im, ok := i.GetCoverThumbnailFromCache(coverID); ok { + if ctx.Err() == nil { + cb(im, nil) + } + } else { + i.fetchAndCacheCoverFromDiskOrServer(ctx, coverID, i.thumbnailCache.DefaultTTL, cb) + } + }() + return cancel +} + +// GetCoverThumbnail is a synchronous, blocking function to fetch the image for a given coverID. +// Like most ImageManager calls, it should usually be called in a goroutine to not block UI loading. func (i *ImageManager) GetCoverThumbnail(coverID string) (image.Image, error) { if im, ok := i.GetCoverThumbnailFromCache(coverID); ok { return im, nil } - return i.fetchAndCacheCoverFromDiskOrServer(coverID, i.thumbnailCache.DefaultTTL) + return i.fetchAndCacheCoverFromDiskOrServer(context.Background(), coverID, i.thumbnailCache.DefaultTTL, nil) } +// GetCoverThumbnailWithTTL fetches the cover for the given coverID and updates the TTL +// in the in-memory image cache. It blocks until the image fetch is complete. func (i *ImageManager) GetCoverThumbnailWithTTL(coverID string, ttl time.Duration) (image.Image, error) { - // in-memory cache if img, err := i.thumbnailCache.GetWithNewTTL(coverID, ttl); err == nil { return img, nil } - return i.fetchAndCacheCoverFromDiskOrServer(coverID, ttl) + return i.fetchAndCacheCoverFromDiskOrServer(context.Background(), coverID, ttl, nil) } +// GetFullSizeCoverArt fetches the full size cover image for the given coverID. +// It blocks until the fetch is complete. func (i *ImageManager) GetFullSizeCoverArt(coverID string) (image.Image, error) { if i.cachedFullSizeCoverID == coverID { i.cachedFullSizeCoverAccessedAt = time.Now().UnixMilli() @@ -108,7 +139,10 @@ func (i *ImageManager) GetFullSizeCoverArt(coverID string) (image.Image, error) if i.s.Server == nil { return nil, errors.New("logged out") } + + i.serverFetchSema <- struct{}{} // acquire im, err := i.s.Server.GetCoverArt(coverID, 0) + <-i.serverFetchSema // release if err != nil { return nil, err } @@ -118,6 +152,7 @@ func (i *ImageManager) GetFullSizeCoverArt(coverID string) (image.Image, error) return im, nil } +// GetCoverArtURL returns the URL for the locally cached cover thumbnail, if it exists. func (i *ImageManager) GetCoverArtUrl(coverID string) (string, error) { path := i.filePathForCover(coverID) if _, err := os.Stat(path); err == nil { @@ -128,10 +163,13 @@ func (i *ImageManager) GetCoverArtUrl(coverID string) (string, error) { return "", errors.New("cover not found") } +// GetCachedArtistImage returns the artist image for the given artistID from the on-disc cache, if it exists. func (i *ImageManager) GetCachedArtistImage(artistID string) (image.Image, bool) { return i.loadLocalImage(i.filePathForArtistImage(artistID)) } +// FetchAndCacheArtistImage fetches the artist image for the given artistID from the server, +// caching it locally if the fetch succeeds. Blocks until fetch is completed. func (i *ImageManager) FetchAndCacheArtistImage(artistID string, imgURL string) (image.Image, error) { im, err := i.fetchRemoteArtistImage(imgURL) if err != nil { @@ -141,9 +179,10 @@ func (i *ImageManager) FetchAndCacheArtistImage(artistID string, imgURL string) return im, nil } +// RefreshCachedArtistImageIfExpired re-fetches the artist image from the server if expired. func (i *ImageManager) RefreshCachedArtistImageIfExpired(artistID string, imgURL string) error { stat, err := os.Stat(i.filePathForArtistImage(artistID)) - if err == nil && time.Since(stat.ModTime()) > CachedImageValidTime { + if err == nil && time.Since(stat.ModTime()) > cachedImageValidTime { _, err = i.FetchAndCacheArtistImage(artistID, imgURL) } return err @@ -167,7 +206,10 @@ func (i *ImageManager) ensureArtistCoverCacheDir() string { } func (i *ImageManager) fetchRemoteArtistImage(url string) (image.Image, error) { + i.serverFetchSema <- struct{}{} // acquire res, err := fyne.LoadResourceFromURLString(url) + <-i.serverFetchSema // release + if err == nil { im, _, err := image.Decode(bytes.NewReader(res.Content())) if err == nil { @@ -178,41 +220,56 @@ func (i *ImageManager) fetchRemoteArtistImage(url string) (image.Image, error) { return nil, err } -func (i *ImageManager) fetchAndCacheCoverFromDiskOrServer(coverID string, ttl time.Duration) (image.Image, error) { +func (i *ImageManager) fetchAndCacheCoverFromDiskOrServer(ctx context.Context, coverID string, ttl time.Duration, cb func(image.Image, error)) (image.Image, error) { // on disc cache - path := i.filePathForCover(coverID) if i.ensureCoverCacheDir() != "" { + path := i.filePathForCover(coverID) if s, err := os.Stat(path); err == nil { go i.checkRefreshLocalCover(s, coverID, ttl) if img, ok := i.loadLocalImage(path); ok { i.thumbnailCache.SetWithTTL(coverID, img, ttl) + if ctx.Err() == nil && cb != nil { + cb(img, nil) + } return img, nil } } } - return i.fetchAndCacheCoverFromServer(coverID, ttl) + // fetch from server + return i.fetchAndCacheCoverFromServer(ctx, coverID, ttl, cb) } -func (i *ImageManager) fetchAndCacheCoverFromServer(coverID string, ttl time.Duration) (image.Image, error) { +func (i *ImageManager) fetchAndCacheCoverFromServer(ctx context.Context, coverID string, ttl time.Duration, cb func(image.Image, error)) (image.Image, error) { if i.s.Server == nil { - return nil, errors.New("logged out") - } - img, err := i.s.Server.GetCoverArt(coverID, coverArtThumbnailSize) - if err != nil { + err := errors.New("logged out") + if ctx.Err() == nil && cb != nil { + cb(nil, err) + } return nil, err } - if i.ensureCoverCacheDir() != "" { - path := i.filePathForCover(coverID) - _ = i.writeJpeg(img, path) + select { + case <-ctx.Done(): + return nil, context.Canceled + case i.serverFetchSema <- struct{}{}: // acquire + img, err := i.s.Server.GetCoverArt(coverID, coverArtThumbnailSize) + <-i.serverFetchSema // release + if err == nil { + if i.ensureCoverCacheDir() != "" { + _ = i.writeJpeg(img, i.filePathForCover(coverID)) + } + i.thumbnailCache.SetWithTTL(coverID, img, ttl) + } + if ctx.Err() == nil && cb != nil { + cb(img, err) + } + return img, err } - i.thumbnailCache.SetWithTTL(coverID, img, ttl) - return img, nil } func (i *ImageManager) checkRefreshLocalCover(stat os.FileInfo, coverID string, ttl time.Duration) { - if time.Since(stat.ModTime()) > CachedImageValidTime { - i.fetchAndCacheCoverFromServer(coverID, ttl) + if time.Since(stat.ModTime()) > cachedImageValidTime { + i.fetchAndCacheCoverFromServer(context.Background(), coverID, ttl, nil) } } diff --git a/ui/widgets/gridview.go b/ui/widgets/gridview.go index 56ce2a2..b878e57 100644 --- a/ui/widgets/gridview.go +++ b/ui/widgets/gridview.go @@ -40,7 +40,7 @@ func (b *BatchingIterator) NextN(n int) []*mediaprovider.Album { type ImageFetcher interface { GetCoverThumbnailFromCache(string) (image.Image, bool) - GetCoverThumbnail(string) (image.Image, error) + GetCoverThumbnailAsync(string, func(image.Image, error)) context.CancelFunc } type GridViewIterator interface { @@ -126,7 +126,7 @@ func NewGridView(iter GridViewIterator, fetch ImageFetcher, placeholder fyne.Res g.createGridWrap() // fetch initial items - g.fetchMoreItems(36) + g.checkFetchMoreItems(36) return g } @@ -148,37 +148,28 @@ func NewGridViewFromState(state *GridViewState) *GridView { } func (g *GridView) Clear() { - if g.fetchCancel != nil { - g.fetchCancel() - g.fetchCancel = nil - } g.stateMutex.Lock() defer g.stateMutex.Unlock() + g.cancelFetch() g.items = nil g.done = true } func (g *GridView) Reset(iter GridViewIterator) { - if g.fetchCancel != nil { - g.fetchCancel() - g.fetchCancel = nil - } g.stateMutex.Lock() + g.cancelFetch() g.items = nil g.done = false g.highestShown = 0 g.iter = iter g.stateMutex.Unlock() - g.fetchMoreItems(36) + g.checkFetchMoreItems(36) g.Refresh() } func (g *GridView) ResetFromState(state *GridViewState) { - if g.fetchCancel != nil { - g.fetchCancel() - g.fetchCancel = nil - } g.stateMutex.Lock() + g.cancelFetch() g.GridViewState = *state g.stateMutex.Unlock() g.grid.Refresh() @@ -187,11 +178,8 @@ func (g *GridView) ResetFromState(state *GridViewState) { } func (g *GridView) ResetFixed(items []GridViewItemModel) { - if g.fetchCancel != nil { - g.fetchCancel() - g.fetchCancel = nil - } g.stateMutex.Lock() + g.cancelFetch() g.items = items g.done = true g.highestShown = 0 @@ -259,32 +247,23 @@ func (g *GridView) doUpdateItemCard(itemIdx int, card *GridViewItem) { } card.Update(item) card.PrevID = item.ID - // cancel any previous image fetch + // cancel any previous image fetch (no issues with possible double-invocations) if card.ImgLoadCancel != nil { card.ImgLoadCancel() - card.ImgLoadCancel = nil } if item.CoverArtID != "" { if img, ok := g.imageFetcher.GetCoverThumbnailFromCache(item.CoverArtID); ok { card.Cover.SetImage(img) } else { card.Cover.SetImage(nil) - // asynchronously fetch cover image - ctx, cancel := context.WithCancel(context.Background()) - card.ImgLoadCancel = cancel - go func(ctx context.Context) { - i, err := g.imageFetcher.GetCoverThumbnail(item.CoverArtID) - select { - case <-ctx.Done(): - return - default: - if err == nil { - card.Cover.SetImage(i) - } else { - log.Printf("error fetching image: %s", err.Error()) - } + card.ImgLoadCancel = g.imageFetcher.GetCoverThumbnailAsync(item.CoverArtID, func(i image.Image, err error) { + if err == nil { + card.Cover.SetImage(i) + } else { + log.Printf("error fetching image: %s", err.Error()) } - }(ctx) + card.ImgLoadCancel() // done. release resources associated with cancel channel + }) } } else { // use the placeholder for an item that has no cover art ID @@ -292,8 +271,8 @@ func (g *GridView) doUpdateItemCard(itemIdx int, card *GridViewItem) { } // if user has scrolled near the bottom, fetch more - if !g.done && g.fetchCancel == nil && itemIdx > g.lenItems()-10 { - g.fetchMoreItems(20) + if itemIdx > g.lenItems()-10 { + g.checkFetchMoreItems(20) } } @@ -303,8 +282,14 @@ func (g *GridView) lenItems() int { return len(g.items) } -// fetches at least count more items -func (g *GridView) fetchMoreItems(count int) { +// fetches at least count more items if fetch not in progress and not done +// acquires stateMutex for atomicity +func (g *GridView) checkFetchMoreItems(count int) { + g.stateMutex.Lock() + defer g.stateMutex.Unlock() + if g.done || g.fetchCancel != nil { + return // done, or fetch already in progress + } if g.iter == nil { g.done = true return @@ -335,10 +320,21 @@ func (g *GridView) fetchMoreItems(count int) { } } } - g.fetchCancel = nil + // call cancelfunc to release Context resources + g.stateMutex.Lock() + g.cancelFetch() + g.stateMutex.Unlock() }() } +// must be called with stateMutex locked for writing +func (g *GridView) cancelFetch() { + if g.fetchCancel != nil { + g.fetchCancel() + g.fetchCancel = nil + } +} + func (g *GridView) showContextMenu(card *GridViewItem, pos fyne.Position) { g.menuGridViewItemId = card.ItemID() if g.menu == nil {