[YouTube] Rewrite PoToken functionality using coroutines#12028
[YouTube] Rewrite PoToken functionality using coroutines#12028Isira-Seneviratne wants to merge 4 commits intoTeamNewPipe:refactorfrom
Conversation
|
ShareASmile
added
feature request
| try { | ||
| return getWebClientPoToken(videoId = videoId, forceRecreate = false) | ||
| } catch (e: RuntimeException) { | ||
| // RxJava's Single wraps exceptions into RuntimeErrors, so we need to unwrap them here | ||
| when (val cause = e.cause) { | ||
| return try { | ||
| runBlocking { getWebClientPoToken(videoId, forceRecreate = false) } | ||
| } catch (e: Exception) { | ||
| when (e) { |
There was a problem hiding this comment.
Is that change to catch okay?
| webPoTokenGenLock.withLock { | ||
| val gen = webPoTokenGenerator |
There was a problem hiding this comment.
With the GuardedByMutex class that I thought up in a8da994 it would be possible to put the generator into the lock, making it impossible to access it without locking first. I think that’s a pretty easy win.
| if (shouldRecreate) { | ||
|
|
||
| val innertubeClientRequestInfo = InnertubeClientRequestInfo.ofWebClient() | ||
| innertubeClientRequestInfo.clientInfo.clientVersion = | ||
| YoutubeParsingHelper.getClientVersion() | ||
|
|
||
| webPoTokenVisitorData = YoutubeParsingHelper.getVisitorDataFromInnertube( | ||
| innertubeClientRequestInfo, | ||
| NewPipe.getPreferredLocalization(), | ||
| NewPipe.getPreferredContentCountry(), | ||
| YoutubeParsingHelper.getYouTubeHeaders(), | ||
| YoutubeParsingHelper.YOUTUBEI_V1_URL, | ||
| null, | ||
| false | ||
| ) | ||
| // close the current webPoTokenGenerator on the main thread | ||
| webPoTokenGenerator?.let { Handler(Looper.getMainLooper()).post { it.close() } } | ||
| webPoTokenVisitorData = withContext(Dispatchers.IO) { | ||
| val innertubeClientRequestInfo = InnertubeClientRequestInfo.ofWebClient() |
There was a problem hiding this comment.
Do I read this correctly, before it was executed blocking on the main thread and now it’s dispatched to the IO thread?
| // any other (player) tokens. | ||
| webPoTokenStreamingPot = webPoTokenGenerator!! | ||
| .generatePoToken(webPoTokenVisitorData!!).blockingGet() | ||
| webPoTokenStreamingPot = webPoTokenGenerator!!.generatePoToken(webPoTokenVisitorData!!) |
There was a problem hiding this comment.
A bit weird that webPaTokenGenerator can be null here? Shouldn’t it be guaranteed to be there?
| """try { | ||
| identifier = "$identifier" | ||
| u8Identifier = $u8Identifier | ||
| u8Identifier = ${stringToU8(identifier)} | ||
| poTokenU8 = obtainPoToken(webPoSignalOutput, integrityToken, u8Identifier) | ||
| poTokenU8String = "" | ||
| for (i = 0; i < poTokenU8.length; i++) { | ||
| if (i != 0) poTokenU8String += "," | ||
| poTokenU8String += poTokenU8[i] | ||
| } | ||
| poTokenU8String = poTokenU8.join(",") | ||
| $JS_INTERFACE.onObtainPoTokenResult(identifier, poTokenU8String) | ||
| } catch (error) { | ||
| $JS_INTERFACE.onObtainPoTokenError(identifier, error + "\n" + error.stack) | ||
| }""", |
There was a problem hiding this comment.
Do not change the javascript in any place. This needs to work on browsers from 20 years ago, which some devices still use as webviews. If you check the PR adding po token support, I specifically worked on making the javascript as old as possible to avoid any incompatibilities, and neither .join() nor stringToU8() are good.
| private val poTokenContinuations = | ||
| Collections.synchronizedMap(ArrayMap<String, Continuation<String>>()) |
There was a problem hiding this comment.
Why do you need a synchronizedMap for this? If the list is only accessed in suspended functions there should be nothing parallel going on, and the ordering of suspend calls should take care of it (as long as everything is run in the same coroutine block)
|
lol there is no way to retract my approval, thank you github. Obviously fix the thing @Stypox mentioned :) |
| * Adds the ([identifier], [continuation]) pair to the [poTokenContinuations] list. This makes | ||
| * it so that multiple poToken requests can be generated in parallel, and the results will be | ||
| * notified to the right continuations. | ||
| */ | ||
| private fun addPoTokenEmitter(identifier: String, emitter: SingleEmitter<String>) { | ||
| synchronized(poTokenEmitters) { | ||
| poTokenEmitters.add(Pair(identifier, emitter)) | ||
| } | ||
| private fun addPoTokenEmitter(identifier: String, continuation: Continuation<String>) { | ||
| poTokenContinuations[identifier] = continuation | ||
| } |
There was a problem hiding this comment.
So as far as I understand, addPoTokenEmitter is called exactly once inside generatePoToken.
generatePoToken is called exactly twice, both times in getWebClientPoToken. And here’s the kicker: in the same suspend block, right underneath each other.
So this leads me to believe that the whole infrastructure of having a list of continuations is not really necessary, because everything happens one after the other anyway? I mean yes, in theory they could run at the same time, but in practice there’s only two calls running, and in sequence.
There was a problem hiding this comment.
What I’m saying here is that rewrite to continuations all well and good, but I think we don’t even need such a complicated setup in the first place.
There was a problem hiding this comment.
Yeah initially I thought it would be possible to calculate both the player and the streaming poTokens in parallel, but in the end this didn't work because the player poToken needs to be generated once before anything else.
|
|
New PoToken method with VİDEO-ID |
5 participants
What is it?
Description of the changes in your PR
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence