Skip to content
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

Skip looking for tracks for queue items in error #1725

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

waldo121
Copy link
Contributor

@waldo121 waldo121 commented Oct 23, 2024

Fixes the issue described in this issue comment. It lead to the queue looping over finding streams for a specific track and being stuck there.
Should the item in error be removed from the queue or leaving it there is ok 🤔?

@nukeop nukeop added the under review This pull request is being reviewed by maintainers. label Oct 23, 2024
@nukeop
Copy link
Owner

nukeop commented Oct 25, 2024

I was thinking about this and it would be good to "lock" some tracks in the queue where they're e.g. DRM-locked and cannot be played, showing that to the user.

Another thing I noticed that sometimes retrying a couple of times succeeds and gets the stream successfully even after an initial failure. So while this behavior is a bug, I think it would be good to reintroduce it as a purposefully added feature (3 retries per track, with exponential backoff, plus UI elements showing this).

@waldo121
Copy link
Contributor Author

waldo121 commented Oct 25, 2024

I was thinking about this and it would be good to "lock" some tracks in the queue where they're e.g. DRM-locked and cannot be played, showing that to the user.

Let me get this straight. The whole streamLookup/play wouldn't even be attempted on a "locked" item and the user should get a visual hint to explain this. The point of this is to prevent search and/or download of content that is not supported by nuclear right?

I think it makes more sense to handle this at the stream level rather than the track level because I guess it would be possible for a track to have some streams that work and some others that don't, specially if several stream providers are involved.

Another thing I noticed that sometimes retrying a couple of times succeeds and gets the stream successfully even after an initial failure. So while this behavior is a bug, I think it would be good to reintroduce it as a purposefully added feature (3 retries per track, with exponential backoff, plus UI elements showing this).

I think it makes sense. I have a proposition about the ui for this. The point is to change the color according to exponential backoff progress. It shows something is happening, it is subtle, and it wouldn't overload the ui with information.
ex2
The question remains, what to do after the 3 tries?
a) remove from queue
b) do nothing
c) ask the user what he wants to do with it
d) lock it

@nukeop
Copy link
Owner

nukeop commented Nov 6, 2024

I think after several tries the track should be rendered in gray (or similar "inactive" color) and there should be a clear message why this happened. I never liked that tracks that keep failing are removed, but I couldn't come up with a better solution.

At the same time after a track is locked you should still be able to select a new stream and start with 3 more retries since as you said, different streams for the same track may behave differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under review This pull request is being reviewed by maintainers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants