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

Delivery methods without design flaws #862

Draft
wants to merge 43 commits into
base: dev
Choose a base branch
from

Conversation

litetex
Copy link
Member

@litetex litetex commented Jun 18, 2022

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • 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:

  • Rebase the PR from dev
  • 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 litetex force-pushed the delivery-methods-without-design-flaws branch 2 times, most recently from dd35950 to fd33006 Compare June 19, 2022 13:10
@litetex litetex added the multiservice Issues related to multiple services label Jun 19, 2022
@litetex litetex force-pushed the delivery-methods-without-design-flaws branch 2 times, most recently from fd52219 to c0f1777 Compare June 20, 2022 19:20
@litetex litetex mentioned this pull request Jun 22, 2022
2 tasks
@litetex litetex force-pushed the delivery-methods-without-design-flaws branch 2 times, most recently from c6b4e06 to d251a91 Compare June 24, 2022 20:56
@Stypox
Copy link
Member

Stypox commented Jul 13, 2022

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.

@litetex
Copy link
Member Author

litetex commented Jul 15, 2022

@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.

@Stypox
Copy link
Member

Stypox commented Aug 24, 2022

@litetex any news? :-)

@litetex litetex force-pushed the delivery-methods-without-design-flaws branch from d0a4a8d to 8b7cb62 Compare August 26, 2022 15:28
@litetex
Copy link
Member Author

litetex commented Aug 26, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multiservice Issues related to multiple services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve handling of HLS/DASH/Progressive HTTP streaming data
2 participants