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

feat(YouTube - Shorts components): add Custom actions in toolbar setting (YouTube 18.38.44+) #106

Merged
merged 5 commits into from
Dec 15, 2024

Conversation

Francesco146
Copy link

@Francesco146 Francesco146 commented Dec 14, 2024

open to changes and suggestions.

  • i hate litho stuff.
Preview
immagine

previously known as: #101

@Francesco146
Copy link
Author

i think the whole process can be simplified using this:

/**
* This fingerprint is compatible with all versions of YouTube starting from v18.29.38 to supported versions.
* This method is invoked only in Shorts.
* Accurate video information is invoked even when the user moves Shorts upward or downward.
*/
internal val videoIdFingerprintShorts = legacyFingerprint(
name = "videoIdFingerprintShorts",
returnType = "V",
parameters = listOf(PLAYER_RESPONSE_MODEL_CLASS_DESCRIPTOR),
opcodes = listOf(
Opcode.INVOKE_INTERFACE,
Opcode.MOVE_RESULT_OBJECT
),
customFingerprint = custom@{ method, _ ->
if (method.containsLiteralInstruction(45365621L))
return@custom true
method.indexOfFirstInstruction {
getReference<FieldReference>()?.name == "reelWatchEndpoint"
} >= 0
}
)

if so, why is it not also used on RYD? why in that case we filter the proto buffer?

@YT-Advanced
Copy link

if so, why is it not also used on RYD? why in that case we filter the proto buffer?

I think the main reason is the delay. RYD data must be fetched as soon as possible

@Francesco146
Copy link
Author

Francesco146 commented Dec 14, 2024

ah okey. then in this context it doesn't make sense to read the id from the buffer, since to open the menu, the id have time to propagate. I'll update the code tomorrow :)

edit:
that fingerprint hooks setVideoInformation, which was used in the very first implementation, but i was told that this information was not reliable due to the pre-loading of multiple Shorts at the same time.

long story short: i need clarification about which implementation to use

@0xrxL
Copy link

0xrxL commented Dec 14, 2024

I'm highly interested to the fourth option (those to open shorts in normal player). Maybe can be implemented in a wat to be triggered at a start of a short.

…n `Enable open links directly` was disabled
@Francesco146 Francesco146 force-pushed the hook-shorts-more-button branch from 31bc9b1 to aef906a Compare December 14, 2024 21:34
@Francesco146
Copy link
Author

[...]. Maybe can be implemented in a wat to be triggered at a start of a short.

by doing that, you would always be interrupted in scrolling through the videos. i don't know how useful something like that could be

@0xrxL
Copy link

0xrxL commented Dec 14, 2024

[...]. Maybe can be implemented in a wat to be triggered at a start of a short.

by doing that, you would always be interrupted in scrolling through the videos. i don't know how useful something like that could be

Wdym?

@0xrxL
Copy link

0xrxL commented Dec 15, 2024

@Francesco146 ok, now I understand what you were referring to before. Clicking the Short button would trigger many instances of normal player, that would prevent you from swiping down to see more shorts. In this case, however, there is already a fingerprint to map the bottom navigation buttons, so when you'll click the shorts button, only shorts view will be opened.

In this way you'll be able to open normal player for shorts, only in feed/search list.

# Conflicts:
#	extensions/shared/src/main/java/app/revanced/extension/youtube/utils/VideoUtils.java
@Francesco146 Francesco146 marked this pull request as draft December 15, 2024 07:14
@Francesco146
Copy link
Author

i marked this pr as a draft because it is still missing some key aspects. i think that instead of many implementations of the same thing, it's not the right choice. there are some bugs to fix and this takes some time

@YT-Advanced
Copy link

YT-Advanced commented Dec 15, 2024

Uhm we should merge all videoId litho filter into something like ShortsVideoIdPatch, since all of these patch do the same functionality (getting Shorts videoId):

  • Shorts RYD
  • Return YouTube Channel Username
  • Custom Action
  • Hook Shorts Toolbar

@inotia00
Copy link
Owner

Considering this PR, dad6b3d was committed

@inotia00 inotia00 marked this pull request as ready for review December 15, 2024 07:34
@inotia00 inotia00 changed the title feat(YouTube - Shorts Player): Hook More button feat(YouTube - Shorts components): add Custom actions in toolbar setting (YouTube 18.38.44+) Dec 15, 2024
@Francesco146
Copy link
Author

i don't think it's a good idea to release this yet, the code is confusing, duplicated and not documented

@inotia00
Copy link
Owner

Refactoring ShortsVideoIdPatch should be done via another PR, but I don't think it's necessary right now

Even if it is implemented, it won't resolve issues like inotia00/ReVanced_Extended#1538 or inotia00/ReVanced_Extended#1547

@Francesco146
Copy link
Author

ok, 9e50e0d broke backgroundPlaybackPatch, be aware of that

@inotia00
Copy link
Owner

Not all the code I'm working on locally has been merged into the dev branch yet

It will be tested for functional issues before a new release

@inotia00 inotia00 merged commit 73b95a5 into inotia00:dev Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants