Newpipe extractor #199

mikooomich · 2025-01-08T22:03:01Z

What is it?

Description of the changes in your PR

Use NewPipe Extractor for playback
Port of Player fixes z-huang/InnerTune#1789

Fixes the following issue(s)

Some work for Error 2004 #150 . This isn't feature parity with innertube before though, as there is no support for private content

Due diligence

I read the contribution guidelines.
I understand that in the event of merge conflicts, I may be asked to rebase my branch on top of the dev branch, instead of resolving them by merging dev into my own branch

gechoto · 2025-01-08T23:13:01Z

settings.gradle.kts

+//includeBuild("../TubularExtractor") {
+//    dependencySubstitution {
+//        substitute(module("com.github.gechoto:NewPipeExtractor"))
+//            .using(project(":extractor"))
+//    }
+//}


Does this work for you without removing the version?
Because for me it doesn't which is one reason why I did not add it.

gechoto · 2025-01-08T23:14:15Z

settings.gradle.kts

+// We assume, that NewPipe and NewPipe Extractor have the same parent directory.
+// If this is not the case, please change the path in includeBuild().
+
+//includeBuild("../TubularExtractor") {


TubularExtractor is out of date and there is nothing needed from it anyway so I think the original NewPipeExtractor should be preferred here instead (also matches the comment above)

gechoto · 2025-01-08T23:16:49Z

settings.gradle.kts

+
+
+// Use a local copy of NewPipe Extractor by uncommenting the lines below.
+// We assume, that NewPipe and NewPipe Extractor have the same parent directory.


Didn't know you renamed OuterTune to NewPipe.
Big announcement when?

gechoto · 2025-01-08T23:26:55Z

funny how you squashed my commits but forgot the most important one:
z-huang/InnerTune@3239896

maybe because it didn't exist a few minutes ago?
nah no excuse let's still put the blame on you

(sorry about the amount of commits)

gechoto · 2025-01-08T23:46:44Z

innertube/src/main/java/com/zionhuang/innertube/InnerTube.kt

+        ytClient(YouTubeClient.WEB_REMIX, true)
        parameter("ver", "2")
        parameter("c", "ANDROID_MUSIC")


is this supposed to be a mix of WEB_REMIX and ANDROID_MUSIC?

maybe just do this:

suspend fun registerPlayback(url: String, cpn: String, playlistId: String?, client: YouTubeClient = YouTubeClient.WEB_REMIX) = httpClient.get(url) { ytClient(client, true) parameter("ver", "2") parameter("c", client.clientName)

It worked correctly, so I kept it as is

I don't think this is a good idea. It just makes the request look very unofficial and suspicious.

That said it should be handled separately. I don't even understand why this change is part of this PR since it has nothing to do with NewPipeExtractor or the player request changes.

gechoto · 2025-01-09T00:07:04Z

innertube/src/main/java/com/zionhuang/innertube/YouTube.kt

        if (playerResponse.playabilityStatus.status == "OK") {
            if (registerPlayback)
                registerPlayback(playlistId, playerResponse.playbackTracking?.videostatsPlaybackUrl?.baseUrl!!)

            return@runCatching playerResponse
        }


I think this is not in a great place. I suggest to make the player and registerPlayback requests two separate independent things.

Registering playback while doing the player request does not make much sense to me because the player request does not guarantee playback and the player request does not depend on registering playback.

Also I think the code would be easier to read when having two separate function calls where needed instead of adding more parameters (like registerPlayback) to the player function.

IMO something like this:

sitDown() eat() drink()

Is more clear than this:

sitDown(alsoEat, alsoDrink)

What do you think about that design?

My preferred solution would be:

In the innertube module:

remove the registerPlayback: Boolean = true parameter from the player function

the player function should only do the player request and not call any other api endpoints unless it depends on them

In the app module:

in places you only need the player response just call the player function

in places you want to register playback call the player function and afterwards the registerPlayback function

the registerPlayback call could be in YTPlayerUtils for example

For the TubularExtractor stuff, I wanted to implement try implementing sponsorblock (... eventually). It was a mostly painless merge to get that up to date.

Does this work for you without removing the version?
Because for me it doesn't which is one reason why I did not add it.

It doesn't, but I'd also want to have it it as an option for future use. I'll put a note about the version thing

I think this is not in a great place. I suggest to make the player and registerPlayback requests two separate independent things.

Yep. Good ideas. Most of this was trying to not refactor the entire code there. For this pr think it's probably best to leave that as is though. You're right, since it doesn't even adhere to how this history is tracked

For the TubularExtractor stuff, I wanted to implement try implementing sponsorblock (... eventually)

Sponsorblock API looks easy enough to integrate I don't it needs a library for that.

Even if you decide to go with TubularExtractor you can switch to it later when needed. But for now NewPipeExtractor makes more sense.

Update: Tubular looks unmaintained now.
Using a dead library is not great.
Just gives you another repo to maintain and makes pulling in or committing upstream fixes take extra steps.

Apart from that I think it makes more sense to have stream extraction and Sponsorblock separated. These are two different things which should be replaceable independent of each other.
When it comes to that TubularExtractor does not follow a good design.

For Sponsorblock I think it should be a module like other data providers already are (see kugou and lrclib) and have nothing to do with innertube or NewPipe stuff.
This way it will be easier to replace only specific modules in case something breaks.

gechoto · 2025-01-09T00:14:52Z

app/src/main/java/com/dd3boh/outertune/utils/YTPlayerUtils.kt

+        playedFormat: FormatEntity?,
+        audioQuality: AudioQuality,
+        connectivityManager: ConnectivityManager,
+        registerPlayback: Boolean = false,


This is currently unused. Maybe this should be used to decide if it should call YouTube.registerPlayback before returning PlaybackData.

mikooomich · 2025-01-09T00:59:04Z

funny how you squashed my commits but forgot the most important one: z-huang/InnerTune@3239896

maybe because it didn't exist a few minutes ago? nah no excuse let's still put the blame on you

(sorry about the amount of commits)

Ah. That came after, yeah. I meant to mark this pr as draft since idk if I want to do a proper rebase and outertune changes on top of your stuff... I got it working and didnt feel like it was time for rebase hell lol

gechoto · 2025-01-09T01:13:25Z

There is one thing still unfinished from the original InnerTune PR and I'm not sure what to do about it:
Proxy support.
z-huang/InnerTune#1789 (comment)

Idk what do you think? Can it be removed or should it be adapted?

mikooomich · 2025-01-09T02:06:02Z

Uhhh I think you're probably more suited to make that call, in all honesty, I really do not enjoy touching innertube, and I don't think newpipe has that figured out either(?)

Personally, I'd say "do the best-est thing". since you're trying to fix playback with Innertune, it's probably best to fix that and worry about proxy later (probably only used by a fraction of the userbase), and then remove the UI parts so it doesn't confuse the user, and then reintroduce proxy the future. Maybe wait for z-huang to re-awaken ¯_(ツ)_/¯

gechoto · 2025-01-09T03:44:22Z

No idea if it is worth it keeping the proxy settings but just to make this complete I added proxy support now to the new http clients.
Feel free to bring it over from my InnerTune PR.
With that I consider this feature complete. Ready. I guess.

This reverts commit 51431f4.

…ctor

YTPlayerUtils: catch & report url validation errors update NewPipeExtractor make `signatureTimestamp` optional and report exception if it could not be parsed move NewPipe functions to `NewPipeUtils` reduce duplicate code in YTPlayerUtils and skip url status validation for last client improved url deobfuscation error handling YouTubeTest: use IOS client update NewPipeExtractor skip WEB_CREATOR client for logged out users TVHTML5 requires login

…ils`

* Adhere to pause listen history for remote content

mikooomich · 2025-01-10T22:39:10Z

app/src/main/java/com/dd3boh/outertune/utils/YTPlayerUtils.kt

+
+
+//
+//    /**


@gechoto I tried doing what you suggested with moving the register playback to here... but to no avail. Do you have any idea why? The decoupling thing however, yeah... that made a lot of sense to do

Commit: WIP: innertube: register playback stuff

This is not what I meant.
The function doing the registerPlayback request is something suited for innertune.
My idea was to just not call/run the function from within innertune but in YTPlayerUtils instead.
There could be a wrapper in YTPlayerUtils for it which reports exceptions just like it is done for getSignatureTimestampOrNull and findUrlOrNull.

(also the wrapper function in YTPlayerUtils could be responsible for checking settings in the future in case you want to add an option for users to enable/disable register playback

gechoto · 2025-01-10T23:58:45Z

@mikooomich should I open a PR which replaces this one but includes all my suggestions?

mikooomich · 2025-01-11T00:26:37Z

@mikooomich should I open a PR which replaces this one but includes all my suggestions?

Yep, it'll be less janky than my attempt 😄

gechoto · 2025-01-11T02:30:52Z

@mikooomich should I open a PR which replaces this one but includes all my suggestions?

Yep, it'll be less janky than my attempt 😄

@mikooomich #202

gechoto reviewed Jan 8, 2025

View reviewed changes

gechoto reviewed Jan 9, 2025

View reviewed changes

mikooomich and others added 8 commits January 9, 2025 20:32

Revert "hax: innertube: Play all songs not logged in"

5a1f83a

This reverts commit 51431f4.

innertube: Update clients & deobfuscate stream urls with NewPipeExtra…

bf8afa0

…ctor

innertube: rework player requests & response handling

1919704

innertube: Adapt to OuterTune

6b8af8b

gradle: Add local extractor override

fa4fd6a

innertube: Move NewPipe init to NewPipeUtils

3f7a35f

innertube: add proxy support to NewPipeDownloaderImpl & `YTPlayerUt…

c2416be

…ils`

mikooomich force-pushed the newpipe-extractor branch from 1959468 to c2416be Compare January 10, 2025 01:32

mikooomich added 2 commits January 10, 2025 12:53

innertube: Use WEB_REMIX for register playback

3d94855

WIP: innertube: register playback stuff

b2e3444

* Adhere to pause listen history for remote content

mikooomich commented Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Newpipe extractor #199

Newpipe extractor #199

mikooomich commented Jan 8, 2025

gechoto Jan 8, 2025

gechoto Jan 8, 2025

gechoto Jan 8, 2025

gechoto commented Jan 8, 2025 •

edited

Loading

gechoto Jan 8, 2025

mikooomich Jan 9, 2025

gechoto Jan 10, 2025 •

edited

Loading

gechoto Jan 9, 2025 •

edited

Loading

mikooomich Jan 9, 2025

gechoto Jan 9, 2025 •

edited

Loading

gechoto Jan 10, 2025 •

edited

Loading

gechoto Jan 9, 2025

mikooomich commented Jan 9, 2025

gechoto commented Jan 9, 2025

mikooomich commented Jan 9, 2025

gechoto commented Jan 9, 2025 •

edited

Loading

mikooomich Jan 10, 2025

gechoto Jan 10, 2025 •

edited

Loading

gechoto commented Jan 10, 2025 •

edited

Loading

mikooomich commented Jan 11, 2025

gechoto commented Jan 11, 2025



		// Use a local copy of NewPipe Extractor by uncommenting the lines below.
		// We assume, that NewPipe and NewPipe Extractor have the same parent directory.

Newpipe extractor #199

Are you sure you want to change the base?

Newpipe extractor #199

Conversation

mikooomich commented Jan 8, 2025

What is it?

Description of the changes in your PR

Fixes the following issue(s)

Due diligence

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gechoto commented Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gechoto Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

gechoto Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gechoto Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

gechoto Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikooomich commented Jan 9, 2025

gechoto commented Jan 9, 2025

mikooomich commented Jan 9, 2025

gechoto commented Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

gechoto Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

gechoto commented Jan 10, 2025 • edited Loading

mikooomich commented Jan 11, 2025

gechoto commented Jan 11, 2025

gechoto commented Jan 8, 2025 •

edited

Loading

gechoto Jan 10, 2025 •

edited

Loading

gechoto Jan 9, 2025 •

edited

Loading

gechoto Jan 9, 2025 •

edited

Loading

gechoto Jan 10, 2025 •

edited

Loading

gechoto commented Jan 9, 2025 •

edited

Loading

gechoto Jan 10, 2025 •

edited

Loading

gechoto commented Jan 10, 2025 •

edited

Loading