Skip to content

Conversation

@mauriciocolli
Copy link
Contributor

@mauriciocolli mauriciocolli commented Jun 21, 2020

I was working in an extractor for Twitch, and this was needed as it only offer HLS streams.

This also fixes some issues that some YouTube videos were having, as they only had DASH streams. We should be able to fix SoundCloud as well now, I'll leave it for later.

  • Add a field describing how a stream is delivered by a service
  • YouTube
    • Improve DASH manifest parsing
    • Ignore streams using the "OTF" delivery format

Closes #273

Some videos, especially those that have low views, don't have the usual
direct links to all streams, this fix it by properly parsing the DASH
manifest from those special cases.
Rely on the DASH manifest streams instead.
Copy link
Collaborator

@opusforlife2 opusforlife2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a review, just curiosity.

this(null, deliveryFormat, format, resolution, false);
}

public VideoStream(String url, String torrentUrl, MediaFormat format, String resolution) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've replaced 4 VideoStreams by 3. Why doesn't this one have a replacement?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what is the OTF delivery format? Can't find any information online whatsoever.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've replaced 4 VideoStreams by 3. Why doesn't this one have a replacement?

It was used only in one place I think, so I just removed it. It doesn't make a difference anyway.

Also, what is the OTF delivery format? Can't find any information online whatsoever.

Yeah, I don't know exactly, didn't dig too much on it, but it seems to be some sort of in-house playback solution by YouTube, it works pretty much like DASH.

final List<AudioStream> audioStreams =
new ArrayList<>(streamInfo.getAudioStreams());

final DashMpdParser.Result result = DashMpdParser.getStreams(streamInfo);
Copy link
Contributor

@yausername yausername Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we call it YoutubeDashMpdParser and move this code to YoutubeStreamExtractor? Let the service extractors do dash manifest parsing if required.
Or should we have a generic parser that can handle all services like youtube-dl does?

@yausername
Copy link
Contributor

A few questions.

  1. Would it make sense to have getHlsUrl and getDashMpdUrl return a list instead of a string?
  2. How about parsing the HLS manifest similar to how we parse DASH to get different representations of the stream. This can help in, for example selecting live stream resolution for youtube.

@wb9688
Copy link
Contributor

wb9688 commented Jun 24, 2020

First of all: I still haven't looked at your implementation.

My idea was to do the following:

  • Add a type attribute to Stream, which could be one of: progressive HTTP, DASH, HLS or (Web)Torrent.
    • This also means that there won't be a torrentUrl attribute anymore, but those will be 2 separate instances of Stream.
  • Add an id attribute to Stream, which ideally has the original ID the service assigns. This ID should be unique for every stream, but when a stream is provided as e.g. both progressive HTTP and WebTorrent, those 2 instances of Stream will be the same.
  • Add a content attribute to Stream, which could be used instead of url for DASH and HLS and contains the actual manifest itself as String.
  • Add every stream (i.e. Representation in case of DASH) as a separate Stream with just the relevant part of the DASH manifest in the content attribute.
  • Change getDashMpdUrl() and getHlsUrl() to return a Stream with either an url to the full manifest (like YouTube) or with content where we merge the different manifests (like SoundCloud when there's both a MP3 HLS URL and an Opus HLS URL).
    • This means we could use those for an "Auto" resolution option.
    • Also rename those functions to something not containing the word URL.
  • Automatically parse the resolution, format, etc from the YouTube streams instead of using our hardcoded ITAG_LIST, which is pretty incomplete.
    • Also add attributes for HDR and 360° videos.
  • Maybe merge AudioStream, VideoStream, and SubtitlesStream into just Stream, with nullable attributes.

This all obviously doesn't need to be done in a single PR.

/**
* A class that is used to represent the way that a streaming service deliver their streams.
*/
public abstract class DeliveryFormat implements Serializable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this class an interface and add a getUrl method? This will save us some casts.

@yausername yausername mentioned this pull request Jul 1, 2020
3 tasks
@clarfonthey
Copy link

@mauriciocolli Any plans to continue work on this?

@wb9688
Copy link
Contributor

wb9688 commented Aug 3, 2020

@clarfon: Also see my #367. I'll do the NewPipe part of it soon (hopefully for v0.20.1).

@Stypox
Copy link
Member

Stypox commented Aug 4, 2021

Closing in favour of #663

@Stypox Stypox closed this Aug 4, 2021
@ShareASmile
Copy link
Collaborator

for the records:-
Support delivery methods other than progressive HTTP implemented by #810

@opusforlife2
Copy link
Collaborator

Why would you necro such an old PR, @ShareASmile!? The PR linked by Stypox is further linked to the merged one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support SoundCloud HLS

7 participants