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

[YouTube] Cache deobfuscation code and more fixes #447

Merged
merged 2 commits into from
Nov 7, 2020

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Oct 30, 2020

YouTube's changes :-D :-D

Apparently YouTube stopped providing obfuscated URLs completely! This solves many problems in the YouTubeStreamExtractor, since now there won't be any "could not deobfuscate ..." exception and loading will be much faster. I don't know the reason for this change, but I ran every YouTubeStreamExtractor*Test more than 50 times each, all without errors :-D. Btw, this PR also includes (as a standalone commit which should then be squashed, as it does not make sense) my first attempt at speeding up the fetching process, i.e. caching player urls, which are now not needed anymore at all.

In more detail:

  • Cache deobfuscation code and only make the request to get it if needed
  • Fix age restricted videos, and correctly determine age limit
  • Solve a couple of Andoid Studio warnings
  • Do not extract subtitles in onFetchPage(), but only when requested
  • Parse upload date for livestreams and ended livestreams
  • Improve falling back to video info page in case something is missing from initialAjaxJson
  • Add assertNotNull on related streams if they are expected in DefaultStreamExtractorTest

Fixes

Testing apk

Since I don't know whether this change is there all over the world, here is an apk for all of you to test. Please try to test it under many different network conditions.
app-debug.zip
@opusforlife2 could you link here users that encountered problems with youtube streams in 0.20.1? So that we have many people to test this ;-)

@Stypox Stypox requested a review from B0pol October 30, 2020 15:39
@opusforlife2

This comment has been minimized.

@opusforlife2
Copy link
Collaborator

Do we have any DASH videos in our tests?

@xibr

This comment has been minimized.

@B0pol

This comment has been minimized.

@Stypox
Copy link
Member Author

Stypox commented Oct 30, 2020

Thank you, that's reproducible. I'll look into it later

@opusforlife2
Copy link
Collaborator

Hi dEvS! Great App! Very nice!!11! New updare wHEN? :D

@Stypox
Copy link
Member Author

Stypox commented Oct 31, 2020

I readded obfuscation code, and added caching for it: app-debug.zip
I ran every test ~120 times, also adding the above video provided by @xibr, and they all passed.

@Stypox Stypox changed the title [YouTube] Remove deobfuscation, urls are not obfuscated by YouTube anymore [YouTube] Cache deobfuscation code and more fixes Oct 31, 2020
@opusforlife2
Copy link
Collaborator

This apk wants to install over the apk from your fix-detail-open branch.

@Stypox
Copy link
Member Author

Stypox commented Oct 31, 2020

This apk wants to install over the apk from your fix-detail-open branch.

Oh, yes, I built it using that base code, hope that's not a problem ;-)

@Stypox Stypox requested review from B0pol and TobiGr and removed request for B0pol October 31, 2020 20:01
@opusforlife2
Copy link
Collaborator

Well, seeing as I'm testing both... it kind of is a problem. 🤐

@xibr
Copy link

xibr commented Nov 1, 2020

So far it works fine.

@cmomoney
Copy link

cmomoney commented Nov 1, 2020

Seems to be working for me. Thanks.

@Stypox
Copy link
Member Author

Stypox commented Nov 2, 2020

An apk containing both VideoDetailFragment fixes TeamNewPipe/NewPipe#4562 and deobfuscation improvements #447, in case someone (@opusforlife2) wants to test both at the same time: app-debug.zip

@Stypox Stypox force-pushed the better-deobfuscation branch from b707faf to f1a982a Compare November 2, 2020 09:50
@opusforlife2
Copy link
Collaborator

Wut bout seprate APK here?

@Stypox
Copy link
Member Author

Stypox commented Nov 2, 2020

Why would you need that? Here you just need to test that all videos work, not if the app runs faster or whatever

@opusforlife2
Copy link
Collaborator

opusforlife2 commented Nov 2, 2020

So I need an apk that is org schabi newpipe debug, or an apk named with this PR's branch name. The APKs you're uploading are named with the fix-video-details branch name, which is another actively updated PR I'm testing as well. So I need to be able to install them side by side. That's what I meant here #447 (comment).

You were using the default debug name earlier. I still have that installed. (The one which crashes on DASH videos.)

@Stypox
Copy link
Member Author

Stypox commented Nov 2, 2020

Here you are: app-debug.zip

@xibr
Copy link

xibr commented Nov 2, 2020

The video cannot be played with toast message "could not play this stream"
https://youtu.be/7TseTFjNd1Y in Release 0.20.2 It works without any problem.

Same issue #3336 ?!

@opusforlife2

This comment has been minimized.

@xibr
Copy link

xibr commented Nov 3, 2020

I cannot play videos that were previously broadcast live.
https://youtu.be/OWJHyYKVcoE
https://youtu.be/dAIBDfzrzJk

The videos open just fine. I just get an error snackbar.

it happens with me only with this apk

Here you are: app-debug.zip

Does not happen on the apk provided in the first comment.

@Stypox
Copy link
Member Author

Stypox commented Nov 3, 2020

@opusforlife2 that's caused by updating to java 8 and will be fixed by @Isira-Seneviratne
@xibr ok, I guess I'll have to revert to the old way of detecting livestreams

@Isira-Seneviratne
Copy link
Member

@Stypox I added a commit fixing the parsing issue to this PR.

@opusforlife2
Copy link
Collaborator

I cannot play videos that were previously broadcast live.

Ah, you're right. I was just assuming that since video details opened fine there was no error. But yeah, trying to play previously live streams fails with that toast.

However, currently live videos play fine, @Stypox.

@Stypox Stypox force-pushed the better-deobfuscation branch from f1a982a to f9ad46f Compare November 4, 2020 14:51
@Stypox
Copy link
Member Author

Stypox commented Nov 4, 2020

@xibr @opusforlife2 I fixed problems with livestreams by reverting the way they are detected:

The two APKs have the same extractor implementation, so test only one of those. If you choose the first one you can test both this changes and those in TeamNewPipe/NewPipe#4562 at once.

@Stypox Stypox force-pushed the better-deobfuscation branch from f9ad46f to 89a77ae Compare November 4, 2020 15:03
@xibr
Copy link

xibr commented Nov 4, 2020

no issues about parse date now, and live streams problems work as well.

@opusforlife2
Copy link
Collaborator

Yup. All 3 kinds of videos work: live streams, previously live streams, and normal videos.

@Stypox
Copy link
Member Author

Stypox commented Nov 4, 2020

This fixes TeamNewPipe/NewPipe#3336, I tested with a livestream that had just ended and, even though such streams are still considered as running livestreams (which are played as DASH by the NewPipe player), there are no errors anymore since the DASH url is extracted correctly and such videos are detected correctly.

@B0pol
Copy link
Member

B0pol commented Nov 4, 2020

TeamNewPipe/NewPipe#3336

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Looks good.
Next time, please make more commits, E.g. "Automatically fix more things" and "fix age restriction extraction" could have been in separate commits. That speeds up the review process.

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.

Unable to view age restricted content "This live event has ended" - Crash
7 participants