NewPipeExtractor icon indicating copy to clipboard operation
NewPipeExtractor copied to clipboard

Delivery methods without design flaws

Open litetex opened this issue 3 years ago • 4 comments

  • [x] I carefully read the contribution guidelines and agree to them.
  • [ ] I have tested the API against NewPipe.
  • [x] I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Fixes #858

Status: Extractor implementation nearly done, changes in Client/NewPipe still have to be implemented

Changelog

This PR reimplements #810.

Short overview the delivery methods work in the first place (click to expand)

DeliveryMethods drawio

Architecture

The architecture how Streams and StreamingData (and the different types of them) work was completely rewritten.

Click to expand architecture overview

NPStreamingArch862 drawio

Note that all "classes" are interfaces except the YT-specific implementation.

  • Currently you have to do a bunch of unusual checks (like if the currentservice is YT or not) to determine which type of streaming data you have and what you should do with it.
  • There are also things like SmoothStreaming or HLSManifestStreams which are never used
  • The DashManifestCreators are static and not very good extendable. As a user you also have to be determined from the outside what to use and it's not ensured that they are handled correctly.
  • Use of inheritance

Changes in detail

  • Extractors now return ALL streams that are found - even if these are duplicated in terms of quality. De-Duplication and filtering should be handled on the client side.
  • MediaFormat was reworked and split into Service specific media formats like AudioMediaFormat, SubtitleMediaFormat and so on.
  • Stream was reworked and split into multiple interfaces
    • Information about delivery data like HLS or DASH was split into a own interface (DeliveryData) and it's subsequent implementations (e.g. DASHUrlDeliveryData, ProgressiveHTTPDeliveryData, ...)
      • This also resulted in the removal of DeliveryType which is no longer required
    • More or less the same happened to the DashManifestCreators, they are now no longer static and can be extended
  • YouTube's Itags were reworked in a similar manner as described in the classes above (checkout ItagFormatRegistry if you want to know more)
  • The quality(width, height, fps) of a video is now encapsulated inside a own class
  • StreamType was removed and replaced by isLive and isAudioOnly inside the StreamInfo class and the extractors
  • Removed ManifestCreatorCache as it's no longer required
  • Adjusted the test accordingly

In total this PR removes nearly 2k LoC (compared to the original PR #810 that's nearly 50% of the added code) and makes the extractor much more object oriented.


Todo:

  • [x] Rebase the PR from dev
  • [x] Check the code there are still TODOs inside, also fix the JavaDoc
  • [ ] Check changes against NewPipe
  • [ ] Maybe cleanup the commits a bit
  • [ ] Followup
    • [ ] Get url's on demand? (e.g. important for YT so not everything is always decoded/decrypted)

litetex avatar Jun 18 '22 20:06 litetex

Took a brief look: I like the structure. I usually tend to avoid having this many abstract things, but in this case there is no way to avoid it because of the complexity of the data. With this structure a user of the extractor would need to use instanceof to decide how to play a Stream he obtained, doesn't he?

Also, what about removing completely the concept of getVideoStreams, getAudioStreams and getAudioVideoStreams and just create a getStreams that returns all of them, and then the library user can just call .filter(VideoStream.class::isInstance).map(VideoStream.class::cast) to filter out what he needs (we could provide this as a utility method)? This way we would avoid duplicate code for extracting the same thing in 3 places, and we would stop having to store temporarily extracted streaming data in fields that use RAM needlessly. Anyway, this is a suggestion for a future PR.

Stypox avatar Jul 13 '22 15:07 Stypox

@Stypox Sound's interesting but as you said I think this is best in a followup.

I'm currently already struggling with NewPipe itself because nearly everything that is player related has to be reworked (which with the fact that I currently don't have much spare time gradually slows down the progress quite a bit).

Btw there will be further unfinished changes inbound here that are mostly download-related e.g. marking TorrentData as not downloadable or how the playback should be resolved in the best way, e.g. preferring the hlsMasterPlaylistUrl or something else for certain streams.

litetex avatar Jul 15 '22 17:07 litetex

@litetex any news? :-)

Stypox avatar Aug 24 '22 12:08 Stypox

@Stypox Current state: I'm still working on it (but I'm currently busy doing other stuff so progress is kind of slow). The extractor side should be nearly finished (did a few redesigns regarding downloads) but the client side still needs major changes. For now I rebased the branch and fixed conflicts from the last few months.

litetex avatar Aug 26 '22 15:08 litetex