-
Notifications
You must be signed in to change notification settings - Fork 37
Merge develop into master: Performance optimizations and feature request improvements #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7401351
00a30e3
b350509
71fa4cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,3 +56,7 @@ | |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,3 +109,7 @@ Join our Discord server for: | |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2340,15 +2340,15 @@ async function getNowPlaying(options = {}) { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| // Only fetch queue if explicitly needed (skip for status polling to avoid choppy playback) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!skipQueue) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| promises.push( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| sonos.getQueue().catch(err => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore queue errors for now-playing | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| sonos.getQueue().catch(err => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore queue errors for now-playing | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| promises.push(Promise.resolve(null)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [state, volume, queue] = await Promise.all(promises); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Fetch queue (next tracks) - only if we have queue data | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -4463,9 +4463,18 @@ async function _add(input, channel, userName) { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| if (state === 'stopped') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('Player stopped - ensuring queue is active source and flushing'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Parallel stop and flush (flush is safe even if not stopped) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Stop any active playback to force Sonos to use queue | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sonos.stop(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 300)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (stopErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore stop errors (might already be stopped) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Stop command result (may already be stopped): ' + stopErr.message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Flush queue to start fresh | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sonos.flush(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 200)); // Reduced from 300ms | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 300)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('Queue flushed and ready'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (flushErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.warn('Could not flush queue: ' + flushErr.message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -4493,8 +4502,9 @@ async function _add(input, channel, userName) { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| // Try to queue the first valid candidate (most relevant result) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let result = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(`Attempting to queue: ${firstCandidate.name} by ${firstCandidate.artist} (URI: ${firstCandidate.uri})`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sonos.queue(firstCandidate.uri); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('Added track: ' + firstCandidate.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('Successfully queued track: ' + firstCandidate.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = firstCandidate; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const errorDetails = e.message || String(e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -4540,7 +4550,72 @@ async function _add(input, channel, userName) { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| // Start playback in background (don't await) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| (async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 300)); // Brief delay for queue to settle | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ensure queue is the active source before starting playback | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Stop any active playback to force Sonos to use queue | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sonos.stop(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (stopErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore stop errors (might already be stopped) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Stop before play (may already be stopped): ' + stopErr.message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Verify queue has items before trying to play (prevents UPnP error 701) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let queueReady = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let retries = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (!queueReady && retries < 5) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const queue = await sonos.getQueue(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (queue && queue.items && queue.items.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| queueReady = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug(`Queue verified: ${queue.items.length} items ready`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug(`Queue not ready yet (attempt ${retries + 1}/5), waiting...`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 300)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| retries++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (queueErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug(`Queue check failed (attempt ${retries + 1}/5): ${queueErr.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 300)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| retries++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!queueReady) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.warn('Queue not ready after 5 attempts, attempting playback anyway'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Try to activate queue by seeking to position 1 (alternative to SetAVTransportURI) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This should activate the queue as the transport source | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Attempting to seek to queue position 1 to activate queue'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Seek to track 1 in the queue to activate it | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sonos.avTransportService().Seek({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| InstanceID: 0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Unit: 'TRACK_NR', | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Target: '1' | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Successfully sought to track 1, queue should be active'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Wait for seek to complete | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (seekErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If seek fails, try alternative: use next() to jump to first track | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Seek failed, trying next() to activate queue: ' + seekErr.message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Jump to first track in queue (this should activate the queue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sonos.next(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Used next() to activate queue'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 300)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (nextErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('next() also failed: ' + nextErr.message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4602
to
+4610
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If seek fails, try alternative: use next() to jump to first track | |
| logger.debug('Seek failed, trying next() to activate queue: ' + seekErr.message); | |
| try { | |
| // Jump to first track in queue (this should activate the queue) | |
| await sonos.next(); | |
| logger.debug('Used next() to activate queue'); | |
| await new Promise(resolve => setTimeout(resolve, 300)); | |
| } catch (nextErr) { | |
| logger.debug('next() also failed: ' + nextErr.message); | |
| // If seek fails, try alternative: carefully use next() to activate queue | |
| logger.debug('Seek failed, considering next() to activate queue: ' + seekErr.message); | |
| try { | |
| // Check queue length before attempting next() to avoid skipping the only track | |
| const queue = await sonos.getQueue(); | |
| const itemCount = queue && queue.items ? queue.items.length : 0; | |
| if (itemCount <= 1) { | |
| logger.debug(`Skipping next() fallback: queue has ${itemCount} item(s), relying on play()`); | |
| } else { | |
| // Jump to next track in queue (this should activate the queue) | |
| await sonos.next(); | |
| logger.debug('Used next() to activate queue'); | |
| await new Promise(resolve => setTimeout(resolve, 300)); | |
| } | |
| } catch (nextErr) { | |
| logger.debug('Queue check or next() fallback failed: ' + nextErr.message); |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple sleep operations create excessive delays when adding tracks to a stopped player. The function includes:
- 500ms wait after initial stop (line 4557)
- Up to 5 retries with 300ms waits each (lines 4566-4582, potential 1500ms)
- 500ms wait after seek/next (lines 4600, 4608)
- Additional 500ms wait before play (line 4616)
This could result in up to 3 seconds of delay before playback starts, which degrades user experience. Consider reducing the cumulative wait times or using exponential backoff with lower initial delays.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,3 +99,7 @@ module.exports = { | |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,3 +147,7 @@ module.exports = { | |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,3 +63,7 @@ module.exports = { | |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The queue verification retry loop attempts to call
sonos.getQueue()up to 5 times even though the PR description mentions this was optimized to prevent choppy playback with large queues. This retry mechanism in the playback path may cause performance issues with queues of 452+ tracks, as each getQueue() call could be expensive. Consider using theskipQueueoptimization here or reducing the number of retry attempts.