ReVanced / revanced-patches

🧩 Patches for ReVanced
https://revanced.app
GNU General Public License v3.0
2.4k stars 277 forks source link

bug: Video buffer settings are not doing anything #1389

Closed LTVA1 closed 1 year ago

LTVA1 commented 1 year ago

Type

Error at runtime

Bug description

Found out on vanced and revanced that no matter what numbers you have in video buffer settings, the actual behaviour is still the default one, so no changes in how long the buffered part is.

Steps to reproduce

  1. Pick up an hour long video
  2. Scroll to random place, pause, wait until it stops downloading stuff, play the video until it stops
  3. Change video buffer settings
  4. Repeat 2. and see that nothing changes

Relevant log output

Emmm

Screenshots or videos

No response

Solution

No response

Additional context

No response

Acknowledgements

LisoUseInAIKyrios commented 1 year ago

Yes it appears the settings no longer do anything. I tried setting all values to 50 minutes (3,000,000 ms), and new videos still load and play immediately.

Of note, the descriptions are also wrong. "Maximum buffer for playback" and "maximum buffer for playback after rebuffer" should both say "minimum buffer".

I tried editing VideoBufferPatch.java and added debug logging to each of the injection points. And after opening a new video the logs show getPlaybackBuffer()and getReBuffer() are called, but getMaxBuffer() is never called.

oSumAtrIX commented 1 year ago

Likely, a fingerprint resolves to the wrong method, theres a couple which are indistinguishable for unique fingerprinting, so a fingerprint was necessary for a different method which referenced the target method in question that is intended to be injected with the call. @LTVA1 please check and report back, if the settings work with Vanced.

LTVA1 commented 1 year ago

@oSumAtrIX I don't know what fingerprint means there, but I have been trying the setting throughout at least 3 different versions of vanced, just checked rn — doesn't work.

oSumAtrIX commented 1 year ago

Report back with the last version of Vanced, where the patch works.

LTVA1 commented 1 year ago

Sadly never had any which had it working. Got first version of vanced at around beginning of 2021, that's the only thing I know.

oSumAtrIX commented 1 year ago

Old versions are hosted on mirros.

LisoUseInAIKyrios commented 1 year ago

I tried using old Vanced, and I don't see any difference when changing the buffer settings there either.

Tried setting minimum buffer to 15 seconds, start playing video, jump forward 10 seconds (using double tap to seek), and it always rebuffers again. This was using 1080p video resolution.

I remember this use to work, and with a really large prebuffer setting the grey buffer area of the seekbar would become huge. Now it seems to prebuffer less than 10 seconds no matter what I change the Vanced/ReVanced settings to.

Maybe YouTube changed how they hand out streaming data. Because as far as I can tell it's broken in both Vanced and ReVanced.

oSumAtrIX commented 1 year ago

You likely tried on a version where this didn't work. Report back which older version works.

LTVA1 commented 1 year ago

@oSumAtrIX works on 15.38.35, although not strictly — with all buffers set to 5 seconds it buffers around 15 seconds, with all to 50 seconds it buffers 45 seconds, so it kinda works? Although it works in bursts — it loads chunk, than idles for several seconds, then loads next chunk. Smarttube next for example loads all the data continuously at max speed available, and its large buffer lasts for about 8 minutes on full hd, so maybe use its code?

oSumAtrIX commented 1 year ago

so maybe use its code?

Not possible/how it works as the code is not being compiled.

LTVA1 commented 1 year ago

Damn, anyway the version clearly works. I let it load until network is idle for 30 seconds, disable wifi and make it play until it starts trying to load next part.

LisoUseInAIKyrios commented 1 year ago

I looked into this a little more, and it's strange.

It doesn't matter what rebuffer settings I use in the ReVanced settings, and YouTube seems to use a different rebuffer amount almost randomly. Sometimes it rebuffers about 10 seconds, sometimes 20 seconds, and sometimes almost 30 seconds. It appears to behave this way with or without the custom buffer patch.

I dug around with a decompiler, but didn't see any obvious patching problem. Except for getmaxbuffer(), the patch injection points are being called, but I think the YouTube client is now dynamically adjusting the buffer/rebuffer amounts or it's some server side behavior that handles the the buffering behavior.

For now, the custom buffer patch can be hidden (remove the @Patch annotation). And the patch can later be re-enabled if the required changes are figured out.

oSumAtrIX commented 1 year ago

It was not functional in Vanced as well, very old versions work

LisoUseInAIKyrios commented 1 year ago

Until someone figures out a fix (assuming this feature is still possible), should this code be deleted, or should the code be kept but hidden (by removing the @patch annotation)?

oSumAtrIX commented 1 year ago

Usually every non working code should be removed. The correct approach of "coming back to it" ks by tracking it in an issue and VCS history. I would believe people asking why it was removed though, something like a deprecation notice could replace the settings page for some time to shed a little bit of light on why it's gone in favour of UX.