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

Improve handling of HLS/DASH/Progressive HTTP streaming data #858

Open
litetex opened this issue Jun 4, 2022 · 2 comments · May be fixed by #862
Open

Improve handling of HLS/DASH/Progressive HTTP streaming data #858

litetex opened this issue Jun 4, 2022 · 2 comments · May be fixed by #862
Labels
enhancement New feature or request multiservice Issues related to multiple services

Comments

@litetex
Copy link
Member

litetex commented Jun 4, 2022

Followup issue of #810

Currently Stream is dependent on service specific stuff (like YT's itag). This is a big problem because sooner or later this class will blow up. It's already confusing enough and this PR makes it worse by adding stuff for the delivery/transmission-method (currently the Stream class can only contain progressive HTTP streaming data).

It's also currently hard to say which service uses which type of streaming and it's not possible to use different delivery methods for the same stream (not sure if this is possible/useful in the first place...).

Also when looking at the PR for NewPipe: We can't just create a specific playbackresolver/mediasource foreach stream type or service, that will be unmaintainable in the long run.

My ideas how to resolve these problems:

What should be fixed anyway

  1. isVideoOnly inside VideoStream should be removed. It's either a VideoStream or a AudioStream or a VideoAudioStream which contains the data of both.
  2. Move the YT specific stuff out of the stream classes. Example resolution could be a YouTubeVideoStream which extends VideoStream. However this may be solved (see below) in an even better way.
  3. I'm not sure how SubtitleStream is supposed to fit into the below constructs, there may be changes required. As far as I have seen subtitles can be delivered e.g. via DASH or in other ways.

Idea 1

Move everything that is delivery specific from Stream-like classes to a separate StreamDeliveryData-like classes:
Example proposal:
NPStreamingArch2 drawio

Idea 2

If - due to currently unknown reasons to me - it's not possible to do Idea 1 we could - instead of nesting (as done in Idea 1) StreamDeliveryData into Stream - simply create them side by side and derive the corresponding implementations from them.
Example proposal:
NPStreamingArch3 drawio

Overview how the delivery methods work:
DeliveryMethods drawio

@litetex litetex added the enhancement New feature or request label Jun 4, 2022
@litetex litetex self-assigned this Jun 4, 2022
@AudricV AudricV added the multiservice Issues related to multiple services label Jun 4, 2022
@litetex
Copy link
Member Author

litetex commented Jun 10, 2022

If anyone is interested in the current progress / what I have done so far, take a look at:
https://github.com/litetex/NewPipeExtractor/tree/delivery-methods-without-design-flaws
Warning: It's currently not compiling 😆

@ghost

This comment was marked as off-topic.

@litetex litetex removed their assignment Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request multiservice Issues related to multiple services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants