diff --git a/README.md b/README.md index bb84615..2cd9278 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,15 @@ random-songs = 50 spinner = '▁▂▃▄▅▆▇█▇▆▅▄▃▂▁' ``` +The song info panel on the queue takes up space, and memory for album art. You can disable this panel with: + +```toml +[ui] +hide-info-panel = true +``` + +If the panel is hidden, no album art will be fetched from the server. + ## Usage ### General Navigation diff --git a/cache.go b/cache.go new file mode 100644 index 0000000..075ec7e --- /dev/null +++ b/cache.go @@ -0,0 +1,122 @@ +package main + +import ( + "time" + + "github.com/spezifisch/stmps/logger" +) + +// Cache fetches assets and holds a copy, returning them on request. +// A Cache is composed of four mechanisms: +// +// 1. a zero object +// 2. a function for fetching assets +// 3. a function for invalidating assets +// 4. a call-back function for when an asset is fetched +// +// When an asset is requested, Cache returns the asset if it is cached. +// Otherwise, it returns the zero object, and queues up a fetch for the object +// in the background. When the fetch is complete, the callback function is +// called, allowing the caller to get the real asset. An invalidation function +// allows Cache to manage the cache size by removing cached invalid objects. +// +// Caches are indexed by strings, because. They don't have to be, but +// stmps doesn't need them to be anything different. +type Cache[T any] struct { + zero T + cache map[string]T + pipeline chan string + quit func() +} + +// NewCache sets up a new cache, given +// +// - a zero value, returned immediately on cache misses +// - a fetcher, which can be a long-running function that loads assets. +// fetcher should take a key ID and return an asset, or an error. +// - a call-back, which will be called when a requested asset is available. It +// will be called with the asset ID, and the loaded asset. +// - an invalidation function, returning true if a cached object stored under a +// key can be removed from the cache. It will be called with an asset ID to +// check. +// - an invalidation frequency; the invalidation function will be called for +// every cached object this frequently. +// - a logger, used for reporting errors returned by the fetching function +// +// The invalidation should be reasonably efficient. +func NewCache[T any]( + zeroValue T, + fetcher func(string) (T, error), + fetchedItem func(string, T), + isInvalid func(string) bool, + invalidateFrequency time.Duration, + logger *logger.Logger, +) Cache[T] { + + cache := make(map[string]T) + getPipe := make(chan string, 1000) + + go func() { + for i := range getPipe { + asset, err := fetcher(i) + if err != nil { + logger.Printf("error fetching asset %s: %s", i, err) + continue + } + cache[i] = asset + fetchedItem(i, asset) + } + }() + + timer := time.NewTicker(invalidateFrequency) + done := make(chan bool) + go func() { + for { + select { + case <-timer.C: + for k := range cache { + if isInvalid(k) { + delete(cache, k) + } + } + case <-done: + return + } + } + }() + + return Cache[T]{ + zero: zeroValue, + cache: cache, + pipeline: getPipe, + quit: func() { + close(getPipe) + done <- true + }, + } +} + +// Get returns a cached asset, or the zero asset on a cache miss. +// On a cache miss, the requested asset is queued for fetching. +func (c *Cache[T]) Get(key string) T { + if v, ok := c.cache[key]; ok { + return v + } + c.pipeline <- key + return c.zero +} + +// Close releases resources used by the cache, clearing the cache +// and shutting down goroutines. It should be called when the +// Cache is no longer used, and before program exit. +// +// Note: since the current iteration of Cache is a memory cache, it isn't +// strictly necessary to call this on program exit; however, as the caching +// mechanism may change and use other system resources, it's good practice to +// call this on exit. +func (c Cache[T]) Close() { + for k := range c.cache { + delete(c.cache, k) + } + c.quit() +} diff --git a/cache_test.go b/cache_test.go new file mode 100644 index 0000000..872fe77 --- /dev/null +++ b/cache_test.go @@ -0,0 +1,208 @@ +package main + +import ( + "testing" + "time" + + "github.com/spezifisch/stmps/logger" +) + +func TestNewCache(t *testing.T) { + logger := logger.Logger{} + + t.Run("basic string cache creation", func(t *testing.T) { + zero := "empty" + c := NewCache( + zero, + func(k string) (string, error) { return zero, nil }, + func(k, v string) {}, + func(k string) bool { return false }, + time.Second, + &logger, + ) + defer c.Close() + if c.zero != zero { + t.Errorf("expected %q, got %q", zero, c.zero) + } + if c.cache == nil || len(c.cache) != 0 { + t.Errorf("expected non-nil, empty map; got %#v", c.cache) + } + if c.pipeline == nil { + t.Errorf("expected non-nil chan; got %#v", c.pipeline) + } + }) + + t.Run("different data type cache creation", func(t *testing.T) { + zero := -1 + c := NewCache( + zero, + func(k string) (int, error) { return zero, nil }, + func(k string, v int) {}, + func(k string) bool { return false }, + time.Second, + &logger, + ) + defer c.Close() + if c.zero != zero { + t.Errorf("expected %d, got %d", zero, c.zero) + } + if c.cache == nil || len(c.cache) != 0 { + t.Errorf("expected non-nil, empty map; got %#v", c.cache) + } + if c.pipeline == nil { + t.Errorf("expected non-nil chan; got %#v", c.pipeline) + } + }) +} + +func TestGet(t *testing.T) { + logger := logger.Logger{} + zero := "zero" + items := map[string]string{"a": "1", "b": "2", "c": "3"} + c := NewCache( + zero, + func(k string) (string, error) { + return items[k], nil + }, + func(k, v string) {}, + func(k string) bool { return false }, + time.Second, + &logger, + ) + defer c.Close() + t.Run("empty cache get returns zero", func(t *testing.T) { + got := c.Get("a") + if got != zero { + t.Errorf("expected %q, got %q", zero, got) + } + }) + // Give the fetcher a chance to populate the cache + time.Sleep(time.Millisecond) + t.Run("non-empty cache get returns value", func(t *testing.T) { + got := c.Get("a") + expected := "1" + if got != expected { + t.Errorf("expected %q, got %q", expected, got) + } + }) +} + +func TestCallback(t *testing.T) { + logger := logger.Logger{} + zero := "zero" + var gotK, gotV string + expectedK := "a" + expectedV := "1" + c := NewCache( + zero, + func(k string) (string, error) { + return expectedV, nil + }, + func(k, v string) { + gotK = k + gotV = v + }, + func(k string) bool { return false }, + time.Second, + &logger, + ) + defer c.Close() + t.Run("callback gets called back", func(t *testing.T) { + c.Get(expectedK) + // Give the callback goroutine a chance to do its thing + time.Sleep(time.Millisecond) + if gotK != expectedK { + t.Errorf("expected key %q, got %q", expectedV, gotV) + } + if gotV != expectedV { + t.Errorf("expected value %q, got %q", expectedV, gotV) + } + }) +} + +func TestClose(t *testing.T) { + logger := logger.Logger{} + t.Run("pipeline is closed", func(t *testing.T) { + c0 := NewCache( + "", + func(k string) (string, error) { return "A", nil }, + func(k, v string) {}, + func(k string) bool { return false }, + time.Second, + &logger, + ) + // Put something in the cache + c0.Get("") + // Give the cache time to populate the cache + time.Sleep(time.Millisecond) + // Make sure the cache isn't empty + if len(c0.cache) == 0 { + t.Fatalf("expected the cache to be non-empty, but it was. Probably a threading issue with the test, and we need a longer timeout.") + } + defer func() { + if r := recover(); r == nil { + t.Error("expected panic on pipeline use; got none") + } + }() + c0.Close() + if len(c0.cache) > 0 { + t.Errorf("expected empty cache; was %d", len(c0.cache)) + } + c0.Get("") + }) + + t.Run("callback gets called back", func(t *testing.T) { + c0 := NewCache( + "", + func(k string) (string, error) { return "", nil }, + func(k, v string) {}, + func(k string) bool { return false }, + time.Second, + &logger, + ) + defer func() { + if r := recover(); r == nil { + t.Error("expected panic on pipeline use; got none") + } + }() + c0.Close() + c0.Get("") + }) +} + +func TestInvalidate(t *testing.T) { + logger := logger.Logger{} + zero := "zero" + var gotV string + expected := "1" + c := NewCache( + zero, + func(k string) (string, error) { + return expected, nil + }, + func(k, v string) { + gotV = v + }, + func(k string) bool { + return true + }, + 500*time.Millisecond, + &logger, + ) + defer c.Close() + t.Run("basic invalidation", func(t *testing.T) { + if c.Get("a") != zero { + t.Errorf("expected %q, got %q", zero, gotV) + } + // Give the callback goroutine a chance to do its thing + time.Sleep(time.Millisecond) + if c.Get("a") != expected { + t.Errorf("expected %q, got %q", expected, gotV) + } + // Give the invalidation time to be called + time.Sleep(600 * time.Millisecond) + if c.Get("a") != zero { + t.Errorf("expected %q, got %q", zero, gotV) + } + }) +} diff --git a/gui_handlers.go b/gui_handlers.go index e0294d2..1c998d6 100644 --- a/gui_handlers.go +++ b/gui_handlers.go @@ -9,6 +9,7 @@ import ( "github.com/gdamore/tcell/v2" "github.com/spezifisch/stmps/mpvplayer" "github.com/spezifisch/stmps/subsonic" + "github.com/spf13/viper" ) func (ui *Ui) handlePageInput(event *tcell.EventKey) *tcell.EventKey { @@ -136,6 +137,9 @@ func (ui *Ui) Quit() { // bad data. Therefore, we ignore errors. _ = ui.connection.SavePlayQueue([]string{"XXX"}, "XXX", 0) } + if !viper.GetBool("ui.hide-song-info") { + ui.queuePage.coverArtCache.Close() + } ui.player.Quit() ui.app.Stop() } diff --git a/page_queue.go b/page_queue.go index 500dcea..aefbf80 100644 --- a/page_queue.go +++ b/page_queue.go @@ -19,6 +19,7 @@ import ( "github.com/spezifisch/stmps/logger" "github.com/spezifisch/stmps/mpvplayer" "github.com/spezifisch/stmps/subsonic" + "github.com/spf13/viper" ) // TODO show total # of entries somewhere (top?) @@ -53,6 +54,8 @@ type QueuePage struct { logger logger.LoggerInterface songInfoTemplate *template.Template + + coverArtCache Cache[image.Image] } var STMPS_LOGO image.Image @@ -68,15 +71,21 @@ func init() { } func (ui *Ui) createQueuePage() *QueuePage { - tmpl := template.New("song info").Funcs(template.FuncMap{ - "formatTime": func(i int) string { - return (time.Duration(i) * time.Second).String() - }, - }) - songInfoTemplate, err := tmpl.Parse(songInfoTemplateString) - if err != nil { - ui.logger.PrintError("createQueuePage", err) + addSongInfo := !viper.GetBool("ui.hide-song-info") + + var songInfoTemplate *template.Template + if addSongInfo { + tmpl := template.New("song info").Funcs(template.FuncMap{ + "formatTime": func(i int) string { + return (time.Duration(i) * time.Second).String() + }, + }) + var err error + if songInfoTemplate, err = tmpl.Parse(songInfoTemplateString); err != nil { + ui.logger.PrintError("createQueuePage", err) + } } + queuePage := QueuePage{ ui: ui, logger: ui.logger, @@ -151,29 +160,81 @@ func (ui *Ui) createQueuePage() *QueuePage { queuePage.queueList.SetSelectionChangedFunc(queuePage.changeSelection) - queuePage.coverArt = tview.NewImage() - queuePage.coverArt.SetImage(STMPS_LOGO) - - infoFlex := tview.NewFlex().SetDirection(tview.FlexRow). - AddItem(queuePage.songInfo, 0, 1, false). - AddItem(queuePage.coverArt, 0, 1, false) - infoFlex.SetBorder(true) - infoFlex.SetTitle(" song info ") - - // flex wrapper - queuePage.Root = tview.NewFlex().SetDirection(tview.FlexColumn). - AddItem(queuePage.queueList, 0, 2, true). - AddItem(infoFlex, 0, 1, false) - // private data queuePage.queueData = queueData{ starIdList: ui.starIdList, } + // flex wrapper + queuePage.Root = tview.NewFlex().SetDirection(tview.FlexColumn). + AddItem(queuePage.queueList, 0, 2, true) + + if addSongInfo { + // Song info + queuePage.songInfo = tview.NewTextView() + queuePage.songInfo.SetDynamicColors(true).SetScrollable(true) + + queuePage.coverArt = tview.NewImage() + queuePage.coverArt.SetImage(STMPS_LOGO) + + infoFlex := tview.NewFlex().SetDirection(tview.FlexRow). + AddItem(queuePage.songInfo, 0, 1, false). + AddItem(queuePage.coverArt, 0, 1, false) + infoFlex.SetBorder(true) + infoFlex.SetTitle(" song info ") + queuePage.Root.AddItem(infoFlex, 0, 1, false) + + queuePage.coverArtCache = NewCache( + // zero value + STMPS_LOGO, + // function that loads assets; can be slow + ui.connection.GetCoverArt, + // function that gets called when the actual asset is loaded + func(imgId string, img image.Image) { + row, _ := queuePage.queueList.GetSelection() + // If nothing is selected, set the image to the logo + if row >= len(queuePage.queueData.playerQueue) || row < 0 { + ui.app.QueueUpdate(func() { + queuePage.coverArt.SetImage(STMPS_LOGO) + }) + return + } + // If the fetched asset isn't the asset for the current song, + // just skip it. + currentSong := queuePage.queueData.playerQueue[row] + if currentSong.CoverArtId != imgId { + return + } + // Otherwise, the asset is for the current song, so update it + ui.app.QueueUpdate(func() { + queuePage.coverArt.SetImage(img) + }) + }, + // function called to check if asset is invalid: + // true if it can be purged from the cache, false if it's still needed + func(assetId string) bool { + for _, song := range queuePage.queueData.playerQueue { + if song.CoverArtId == assetId { + return false + } + } + // Didn't find a song that needs the asset; purge it. + return true + }, + // How frequently we check for invalid assets + time.Minute, + ui.logger, + ) + } + return &queuePage } func (q *QueuePage) changeSelection(row, column int) { + // If the user disabled song info, there's nothing to do + if q.songInfo == nil { + return + } q.songInfo.Clear() if row >= len(q.queueData.playerQueue) || row < 0 || column < 0 { q.coverArt.SetImage(STMPS_LOGO) @@ -182,15 +243,7 @@ func (q *QueuePage) changeSelection(row, column int) { currentSong := q.queueData.playerQueue[row] art := STMPS_LOGO if currentSong.CoverArtId != "" { - if nart, err := q.ui.connection.GetCoverArt(currentSong.CoverArtId); err == nil { - if nart != nil { - art = nart - } else { - q.logger.Printf("%q cover art %s was unexpectedly nil", currentSong.Title, currentSong.CoverArtId) - } - } else { - q.logger.Printf("error fetching cover art for %s: %v", currentSong.Title, err) - } + art = q.coverArtCache.Get(currentSong.CoverArtId) } q.coverArt.SetImage(art) _ = q.songInfoTemplate.Execute(q.songInfo, currentSong) diff --git a/subsonic/api.go b/subsonic/api.go index 9c64041..0acea6b 100644 --- a/subsonic/api.go +++ b/subsonic/api.go @@ -35,7 +35,6 @@ type SubsonicConnection struct { logger logger.LoggerInterface directoryCache map[string]SubsonicResponse - coverArts map[string]image.Image } func Init(logger logger.LoggerInterface) *SubsonicConnection { @@ -45,7 +44,6 @@ func Init(logger logger.LoggerInterface) *SubsonicConnection { logger: logger, directoryCache: make(map[string]SubsonicResponse), - coverArts: make(map[string]image.Image), } } @@ -384,18 +382,14 @@ func (connection *SubsonicConnection) GetMusicDirectory(id string) (*SubsonicRes return resp, nil } -// GetCoverArt fetches album art from the server, by ID. The results are cached, -// so it is safe to call this function repeatedly. If id is empty, an error -// is returned. If, for some reason, the server response can't be parsed into -// an image, an error is returned. This function can parse GIF, JPEG, and PNG -// images. +// GetCoverArt fetches album art from the server, by ID. If id is empty, an +// error is returned. If, for some reason, the server response can't be parsed +// into an image, an error is returned. This function can parse GIF, JPEG, and +// PNG images. func (connection *SubsonicConnection) GetCoverArt(id string) (image.Image, error) { if id == "" { return nil, fmt.Errorf("GetCoverArt: no ID provided") } - if rv, ok := connection.coverArts[id]; ok { - return rv, nil - } query := defaultQuery(connection) query.Set("id", id) query.Set("f", "image/png") @@ -433,10 +427,6 @@ func (connection *SubsonicConnection) GetCoverArt(id string) (image.Image, error default: return nil, fmt.Errorf("[%s] unhandled image type %s: %v", caller, res.Header["Content-Type"][0], err) } - if art != nil { - // FIXME connection.coverArts shouldn't grow indefinitely. Add some LRU cleanup after loading a few hundred cover arts. - connection.coverArts[id] = art - } return art, err }