Fix duplicate items in queue causing endless buffering #6712
Conversation
|
I just tested it and the bug was fixed, thank you very much!
|
| @@ -64,20 +64,6 @@ private PlayQueueItem(@Nullable final String name, @Nullable final String url, | |||
| this.recoveryPosition = RECOVERY_UNSET; | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean equals(final Object o) { | |||
There was a problem hiding this comment.
Oh my! what a bug xD
Did you check the usages of the method? Maybe someone implemented this on purpose.
There was a problem hiding this comment.
Android Studio's "Show usages" yields all of the usages of the equals() method on any object of any class, so that's not much useful. I tried to manually look for usages by checking out the methods in PlayQueue and in Player that use PlayQueueItem comparison and could not find any issue.
The only downside I can think of is that if the same video is queued multiple times in a row, the loading strategy may start to load the current and the next stream contemporarily, but since those streams point to the same url but do not compare equal, they will be loaded twice. But this only happens if two play queue items with the same url start loading contemporarily, because otherwise the caching method prevents reloading something that already was loaded. I don't think this is really a downside.
|
It's finally working, thank you! |
|
"for some reason" :D you have added this yourself Stypox: PR #4562 |
|
@Redirion 🙈🙈
Now I have to understand what I meant 😅😅 |
From TeamNewPipe#4562: Disable player stream preloading only if the current stream is going to be replaced for sure (see this). equals() was implemented for PlayQueueItems, so that (only) the url is compared when checking them.
Stypox
force-pushed
the
fix-duplicate-items-queue
branch
from
2306519
to
736cefe
Compare
|
@Redirion take a look at the last 2 commits, and thank you for taking a look ;-) |
|
@Stypox Was this included in the last update (v0.21.8)? I just tested this version but the bug still exists. |
|
@RickyM7 no, 0.21.8 was a hotfix, just read the changelog |
Got it, thank you! |
|
Many thanks for this so needed fix! |
What is it?
Description of the changes in your PR
This is a simple bugfix that allows having play queues with duplicate items. For some reason an
equals()method was implemented inPlayQueueItemthat only compared the url, and since duplicate items have the same url they were considered the exact samePlayQueueItems. Using Java's defaultequals()method, that just checks if two objects are the same with==, solves the issue, since duplicate items were anyway created one by one and thus are never identical. Removingequals()should be completely fine sincePlayQueueItems are neverclone()d, and therefore we never end up with two objects that should be considered the same but down in memory aren't because they were cloned.Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
I am not completely sure that this doesn't introduce regressions, so it needs some testing. @ekiniko12 @RickyM7 @sqtvrnqliq could you check if the endless buffering on duplicate videos in queue is solved?
Due diligence