-
-
Notifications
You must be signed in to change notification settings - Fork 516
Add a proper support of other delivery methods than progressive HTTP #663
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
Conversation
dd8cac4 to
7564042
Compare
07a2da9 to
1c763a7
Compare
Stypox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me, though this will require testing many videos
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/stream/SubtitlesStream.java
Outdated
Show resolved
Hide resolved
|
Oh, a question: the |
1c763a7 to
3650265
Compare
e39ce8e to
f31a7b8
Compare
.../src/main/java/org/schabi/newpipe/extractor/services/youtube/ContentAndItagItemAndIsUrl.java
Outdated
Show resolved
Hide resolved
Stypox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, except for some (probably unavoidable) code duplication :-)
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
TobiGr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice structure!
I reviewed the first half of the files. I'll try to take a look at the others on the weekend.
extractor/src/main/java/org/schabi/newpipe/extractor/stream/AudioStream.java
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/stream/AudioStream.java
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/stream/AudioStream.java
Outdated
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/stream/StreamInfo.java
Outdated
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Show resolved
Hide resolved
71ebbf6 to
43e0e6a
Compare
|
Note that the CI is failing only because of the |
TobiGr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went through the remaining files and only found one minor thing.
extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/ItagItem.java
Outdated
Show resolved
Hide resolved
d60743d to
eb03497
Compare
7f19059 to
a138daf
Compare
b29f53c to
7531650
Compare
.../org/schabi/newpipe/extractor/services/media_ccc/extractors/MediaCCCLiveStreamExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/stream/StreamInfo.java
Outdated
Show resolved
Hide resolved
.../test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeDashManifestCreatorTest.java
Outdated
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/peertube/extractors/PeertubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
|
Does it make a difference to use |
…improve YoutubeParsingHelper The hardcoded client version used was outdated and buffering was present on a lot of streams so it seems we need to always get the most recent WEB client version; otherwise streams will be throttled. The hardcoded clients version have been updated to their latest version. In YoutubeParsingHelper: - unused methods have been removed; - documentations have been improved; - a lot of warnings have been fixed; - the extraction of the client version has been improved/fixed in terms of result and performance (the shortener version was returned even the standard client version was found). A parameter has been added to streaming URLs from the WEB client: the cver param, which appends the client version to the streaming URL. This param is added by YouTube web players, so let's use it to spoof better the official web clients. Some duplicated code (in YoutubeParsingHelper and YoutubeMusicSearchExtractor) has been merged into one common method named getYoutubeMusicHeaders, located in YoutubeParsingHelper.
…DASH manifests Add the rn param to OTF and post live DVR streaming URLs (rn means request number). This commit also improves documentation of YoutubeDashManifestCreator. YoutubeDashManifestCreatorTest has been also updated with the changes made in YoutubeDashManifestCreator.
Trying to prevent more throttling when streaming ended livestreams or OTF streams by using first streams of the Android client.
…ive streams and massive improvements to YoutubeDashManifestCreator Generate DASH manifests for YouTube progressive streams like Invidious does, fix some bugs, add documentation everywhere in the class. Relevant test class has been updated. For more detailed changes, look at code changes.
… DASH URLs extraction Also improve a bit the code of the files changed with this PR.
… length The value expected is a long so an implicit conversion was done and on bigger videos, the content length value is more than the capacity of an integer, which was making the extractor crash when trying to get video/audio streams. This issue is now fixed with this commit.
Apply opusforlife2's suggestions for comments and JavaDocs of YoutubeDashManifestCreator.
The boolean keyAndVersionExtracted in YoutubeParsingHelper was not set to false when resetting the client version and the key, which makes the extractor uses null on the next getting of the client version or the key if the clientVersion and the key were extracted before.
…reams on long video streams Also apply suggested changes. Co-authored-by: TobiGr <[email protected]>
Also make parameters in setters final.
…ove some code In order to keep compability of the extractor for Android KitKat, we can't use StandardCharsets.UTF_8 but only the UTF_8 string. Also improve some code: remove some duplication and fix some bugs. For more detailed changes, check out code changes.
This field, made private, is recommended to be added by the Java serialization specification.
…hods and add documentation to the DeliveryMethod enum It is named SS in the DeliveryMethod enum, because SmoothStreaming is a bit long word and we are already using abbreviations with DASH and HLS delivery methods. Also add documentation of delivery methods of the enum constants and of the enum itself.
Documentation has been added for the enum constants and for the enum itself.
… resolution and isVideoOnly fields in VideoStream but deprecate their use Getters should be used instead of the fields, like for the others. Return a new constant (one for the AudioStream class, one for the VideoStream class, but they have the same value (because this method is not implemented in the Stream class)) for streams which don't have an itag instead of 0: ITAG_NOT_AVAILABLE_OR_NOT_APPLICABLE, which is equal to -1 (because an itag id should be not negative). Also fix some small issues in the Stream class.
624ffdf to
bc33879
Compare
|
Closing in favor of #810. |
This is the work made by @wb9688 (thank you for your work, I wouldn't have been able to do it myself) in #367 but rebased and with some changes:
Changes in Stream classes
url(now deprecated) tocontentand addisUrlto indicate whether content contains the URL or the file itself (which is useful when we have to split DASH/HLS manifests);idto identify a specific stream (there could be multiple Streams with the same ID if the same stream is offered through multiple delivery methods; in YouTube's case the ID would be the itag);baseUrlfield, required for some players (like ExoPlayer) to play manifests.Other changes:
DeliveryFormatenum withPROGRESSIVE_HTTP,DASH,HLSandTORRENTvalues;ItagItemclass;DefaultStreamExtractorTesttest with the changes made in Stream classes;SoundcloudStreamExtractortest to test if streams returned by the extractor are progressive HTTP (test will fail if that's not the case);POST_LIVE_STREAMandPOST_LIVE_AUDIO_STREAMto StreamType enum;Fixed issues
Closes #461, fixes #648