-
Notifications
You must be signed in to change notification settings - Fork 420
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
Add a proper support of other delivery methods than progressive HTTP #663
Conversation
dd8cac4
to
7564042
Compare
07a2da9
to
1c763a7
Compare
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
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
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/Stream.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 |
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) tocontent
and addisUrl
to indicate whether content contains the URL or the file itself (which is useful when we have to split DASH/HLS manifests);id
to 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);baseUrl
field, required for some players (like ExoPlayer) to play manifests.Other changes:
DeliveryFormat
enum withPROGRESSIVE_HTTP
,DASH
,HLS
andTORRENT
values;ItagItem
class;DefaultStreamExtractorTest
test with the changes made in Stream classes;SoundcloudStreamExtractor
test to test if streams returned by the extractor are progressive HTTP (test will fail if that's not the case);POST_LIVE_STREAM
andPOST_LIVE_AUDIO_STREAM
to StreamType enum;Fixed issues
Closes #461, fixes #648