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(Instagram): Added few patches #3604

Draft
wants to merge 20 commits into
base: dev
Choose a base branch
from

Conversation

swakwork
Copy link

@swakwork swakwork commented Sep 1, 2024

Hi team,

I have added the following patches to Instagram

  • Added Enable developer menu patch
  • Added Max media quality patch
  • Added Selectable bio patch
  • Added Sanitize sharing links patch
  • Added Open links in external browser patch

I have tested these with the latest alpha build(348.0.0.0.42) , at the time, and the patches work fine.

These patches go along with my integrations PR #685

This is my first PR to this project, so do let me know if any code correction is needed.

Thanks,

@cyberboh
Copy link

cyberboh commented Sep 1, 2024

Does possible to download media like on Twitter (X) with long press media and download them?

@oSumAtrIX
Copy link
Member

The added patches don't add a download patch.

Copy link
Member

@oSumAtrIX oSumAtrIX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Couple of small issues that need to be addressed before merging though. I have commented in some places but other applicable places should be considered as well.

@oSumAtrIX oSumAtrIX marked this pull request as draft September 1, 2024 15:08
@oSumAtrIX
Copy link
Member

PRs are squashed to one commit. Either separate each patch to its own PR, or squash them locally into commits called "feat(Instagram): Added Patch name patch" so we can merge as a rebase.

@cyberboh
Copy link

cyberboh commented Sep 1, 2024

The added patches don't add a download patch.

I thought max media patch is save media into high quality.
Could be he can add patch for it. :)

@oSumAtrIX
Copy link
Member

No it just changes the media quality

@swakwork
Copy link
Author

swakwork commented Sep 1, 2024

PRs are squashed to one commit. Either separate each patch to its own PR, or squash them locally into commits called "feat(Instagram): Added Patch name patch" so we can merge as a rebase.

oh alright, I wasnt aware of it. Lemme correct the code base with the above mentioned corrections and create PRs individually. We can probably close this one (then).

@oSumAtrIX
Copy link
Member

Working in a single pr is easier (also to review). We can decide that once all codebase issues are resolved 👍

@swakwork
Copy link
Author

swakwork commented Sep 2, 2024

Hi,

Updated the codebase with the mentioned comments. Let me know if it looks good now.
Also thanks for suggesting "Ktlint".

@oSumAtrIX
Copy link
Member

The review will be iterative, addressing big issues, once solved, smaller issues and so on, until everything looks right. Hope it doesn't cause too much trouble.

@swakwork
Copy link
Author

swakwork commented Sep 2, 2024

Updated the Sanitize sharing links patch to support profile and highlights links.
Do let me know if any more correction is required and also if Max media quality patch is not a worth the time, so that I can remove it

@kazimmt
Copy link
Contributor

kazimmt commented Sep 3, 2024

Does possible to download media ...

@crimera was working on download patch. But he didn't complete the job yet.
crimera/piko#222

@cyberboh
Copy link

cyberboh commented Sep 3, 2024

Does possible to download media ...

@crimera was working on download patch. But he didn't complete the job yet. crimera/piko#222

Maybe he can collaborate to piko in order to implemented it

@cyberboh
Copy link

cyberboh commented Sep 3, 2024

also if Max media quality patch is not a worth the time, so that I can remove it

Better to retain it. It's good too see high quality image.
Just add an info about Max media could be needs more data internet

@oSumAtrIX
Copy link
Member

@swakwork You can mark reviews as resolved to hide them.

@swakwork
Copy link
Author

swakwork commented Sep 6, 2024

@swakwork You can mark reviews as resolved to hide them.

yup, done 👍

@swakwork
Copy link
Author

Hi team, is there any other cleanups required from my end or the changes looks good ?

@oSumAtrIX oSumAtrIX self-requested a review September 14, 2024 14:59
@LisoUseInAIKyrios LisoUseInAIKyrios marked this pull request as ready for review September 14, 2024 16:04
)
@Suppress("unused")
object SelectableBioPatch : BytecodePatch(
setOf(SelectableBioFingerprint),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please name fingerprints to the method they match to. If this one for example matches to the constructor, call it "ConstructBioViewFingerprint"

}.name

// Patch the method that opens the link in the internal browser.
it.mutableClass.methods.last { it.returnType == "V" && it.parameters.size == 0 }.apply {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a safe approach. Please fingerprint the method and get it through that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically that method originally has 4 lines, 2 const/4, 1 invoke and return-void. Moreover both the "get url" method and the method where the fragment is built is present in the same call. So I thought its easier to find the class and then find the methods accordingly


import app.revanced.patcher.fingerprint.MethodFingerprint

internal object OpenLinksInExternalBrowserFingerprint : MethodFingerprint(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name the fingerprint like explained above.

Comment on lines +28 to +32
StoryShareUrlFingerprint,
LiveShareUrlFingerprint,
PostShareClassFinderFingerprint,
ProfileShareUrlFingerprint,
HighlightsShareUrlFingerprint,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fingerprints need to be named according to the method they match too


override fun execute(context: BytecodeContext) {
fun sanitizeUrl(method: MutableMethod) = method.apply {
val index = getInstructions().first { it.opcode == Opcode.IPUT_OBJECT }.location.index - 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment what this index does in the code

Comment on lines +64 to +72
PostShareClassFinderFingerprint.resultOrThrow().mutableMethod.let { method ->
val classIndex = method.getInstructions().last { it.opcode == Opcode.CONST_CLASS }.location.index
val className = method.getInstruction(classIndex).getReference<TypeReference>()!!.type
val parseJsonMethod = context.findClass(className)!!.mutableClass.methods.first {
it.name == "parseFromJson"
}

sanitizeUrl(parseJsonMethod)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't you using a fingerprint for this?

Comment on lines +35 to +36
val maxPostSize = "0x800" // Decimal value = 2048. Maximum post size.
val maxBitRate = "0x989680" // Decimal value = 10000000. Maximum bit rate possible (found in code).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iirc you can use decimals just fine in smali, no need to convert to hex

Comment on lines +41 to +42
DisplayMetricsFingerprint.resultOrThrow().let { it ->
it.mutableMethod.apply {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do not use the result, simply do DisplayMetricsFingerprint.resultOrThrow().mutableMethod.

Comment on lines +60 to +62
it.mutableClass.apply {
val mediaSetMethod =
methods.first { it.returnType == "Lcom/instagram/model/mediasize/ExtendedImageUrl;" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not fingerprint this method in the first place?

// This section of code sets the bitrate of the stories to the max possible.
StoryMediaBitrateFingerprint.resultOrThrow().let { it ->
it.mutableMethod.apply {
val ifLezIndex = getInstructions().first { it.opcode == Opcode.IF_LEZ }.location.index
Copy link
Member

@oSumAtrIX oSumAtrIX Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use opcode patterns in fingerprint to get the index to instructions. This applies to all the other places where you manually try to find an index. Manually only makes sense if you cant fingerprint the index of a pattern properly for example due to repeating patterns.

Copy link
Member

@oSumAtrIX oSumAtrIX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some refactoring. You should check if everything works on your end too. Apart from the pending reviews, looks good otherwise.

}

// Improve quality of reels.
// In general Instagram tend to set the minimum bitrate between max possible and compressed video's bitrate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

End sentences with a dot.

}

// Improve quality of reels.
// In general Instagram tend to set the minimum bitrate between max possible and compressed video's bitrate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't short words such as "max"

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