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

bug(youtube/remember-video-quality): default video quality overwrites manually selected quality when app is closed and reopened during video playback #1042

Closed
3 tasks done
HypeMaxers opened this issue May 2, 2023 · 5 comments · Fixed by ReVanced/revanced-patches-template#2112
Labels
Bug report Something isn't working

Comments

@HypeMaxers
Copy link

HypeMaxers commented May 2, 2023

Type

Other

Bug description

Using the remember video quality patch and saving a specific quality would overwrite any custom quality when the video is paused and the app is sent to the background as shown in the attached video

Previous behavior the patch wouldn't affect the speed if you pause the video.

Steps to reproduce

  1. Patch ReVanced youtube using the latest patches
  2. Choose a default quality in ReVanced setting
  3. ensure that 'remember video quality' setting is turned off
  4. Open a video, and change the quality manually for that video
  5. Pause the video and send the app to the background
  6. Reopen the app, resume the video
  7. Video quality is reset back to the default (and is not the manually chosen quality)

Edit: this same bug is also present when performing the same steps with a default playback speed and remember-playback-speed is turned off.

Screenshots or videos

https://media.discordapp.net/attachments/954147049832603748/1102995631657725982/Screen_Recording_20230502_192043.mp4

Acknowledgements

  • I have searched the existing issues and this is a new and no duplicate or related to another open issue.
  • I have written a short but informative title.
  • I filled out all of the requested information in this issue properly.
@HypeMaxers HypeMaxers added the Bug report Something isn't working label May 2, 2023
@LisoUseInAIKyrios
Copy link
Contributor

LisoUseInAIKyrios commented May 2, 2023

The behavior you describe is a known limitation of the video id hook:

https://github.com/revanced/revanced-integrations/blob/12c7c6844b70d29d40f45bf9548bdabce14fa85b/app/src/main/java/app/revanced/integrations/patches/playback/quality/RememberVideoQualityPatch.java#L148-L159

Currently the hook is called when a video opens, but also many other times (such as opening/closing the app, turning screen on/off, and probably other times too).

The best solution is to improve the video id hook so it's called only once when a video is opened or reopened. Then patches like remember video quality can better discern when to reset the video quality, and when not to.

I'm not sure which prior version you're speaking of, and the only change is when ReturnYouTubeDislike added shorts support (the video id hook was changed then). That happened many months ago. The prior video id hook has the same problem as the current one, but it may have less false positives.

A short term improvement is change the remember video quality patch to use the old prior video id hook (since the newer hook that supports shorts is not useful for remember patches, shorts don't have selectable quality or speed). But this alone will not entirely fix the behavior.

@HypeMaxers
Copy link
Author

HypeMaxers commented May 2, 2023

I'm not sure which prior version you're speaking of

It was working fine when the recommended version was on 18.05.40. Noticed it when I updated past that version

@LisoUseInAIKyrios
Copy link
Contributor

LisoUseInAIKyrios commented May 2, 2023

Perhaps the video id hook is called more often in the versions of YouTube past 18.05.40, but the patch limitation and current behavior has existed since the start of ReVanced.

@oSumAtrIX
Copy link
Member

A trivial workaround would be to check if the video id has changed. If it is the same, return, and do not set the quality remembered. On another side, YouTube has a model that is parsed after the playback descriptor model is sent. The model includes the video id. This could be used to check if a new video has started, but it might be too early to be useful. For example, the objects to set the quality might not be present at that moment.

@LisoUseInAIKyrios
Copy link
Contributor

if the video quality is reset after the video id changes, it would fix the issue of resetting after closing/opening the app, but introduce a new problem:

If the user closes a video and then reopens the same video again, the default quality will not be set.

If a user pays for data and has a low bitrate set as default (to save $$$), then not setting the default in this case would be much worse than the existing limitation of always resetting quality after closing/opening the app.

An alternative solution without changing the video id patch, is add another hook that is called when the video player is closed. Then the remember patch can check for changes to video id, and clear it's video id when the player is closed. That would 100% fix the issue.

@LisoUseInAIKyrios LisoUseInAIKyrios changed the title bug: Remember video quality overwrites the custom quality after the video is paused bug(youtube/remember-video-quality): remember video quality overwrites manually selected quality when app is closed and reopened May 9, 2023
@LisoUseInAIKyrios LisoUseInAIKyrios changed the title bug(youtube/remember-video-quality): remember video quality overwrites manually selected quality when app is closed and reopened bug(youtube/remember-video-quality): default video quality overwrites manually selected quality when app is closed and reopened May 9, 2023
@LisoUseInAIKyrios LisoUseInAIKyrios changed the title bug(youtube/remember-video-quality): default video quality overwrites manually selected quality when app is closed and reopened bug(youtube/remember-video-quality): default video quality overwrites manually selected quality when app is closed and reopened during video playback May 9, 2023
@oSumAtrIX oSumAtrIX transferred this issue from ReVanced/revanced-patches-template Dec 14, 2023
@oSumAtrIX oSumAtrIX transferred this issue from another repository Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug report Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants