Skip to content

cli: add support for stream manifest URL output #3300

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

Merged
merged 1 commit into from
Nov 1, 2020

Conversation

bastimeyer
Copy link
Member

Resolves #3098

This enables the output of the input-stream's manifest/master URL via
streamlink --stream-url URL / streamlink --json URL
in addition to the already supported output when selecting a stream via
streamlink --stream-url URL STREAM / streamlink --json URL STREAM

This is primarily helpful for HLS streams with a variant playlist.


Things to note:

  • This ignores cases where a plugin yields streams from different sources. Only the last stream in the returned dict/OrderedDict gets used, which is usually best or best-unfiltered. --stream-types can still be used though for filtering out unwanted stream types.
  • I didn't add the manifest/master URL to the HLSStream's string representation and own JSON output, as it'd spam the output by a lot. Maybe it should be added though, at least to the JSON output, as it would (partly) solve the issue mentioned in the previous point.
  • Added tests for both handle_url and handle_stream (output without and with stream/quality selection)
  • IMO, streamlink_cli.main.handle_url needs to be refactored, because patching streamlink_cli.main.args is awkward and lots of unrelated args need to be set. The entire main CLI module needs to be simplified and modularized.
  • I'm not sure why the other test methods don't mock the globals in a context manager. That introduces side-effects, no?

@back-to
Copy link
Collaborator

back-to commented Oct 28, 2020

--stream-types can still be used though for filtering out unwanted stream types.

basically required

streamlink https://www.zdf.de/nachrichten/heute-sendungen/so-wird-das-wetter-102.html --stream-priority hls --json --stream-url

vs

streamlink https://www.zdf.de/nachrichten/heute-sendungen/so-wird-das-wetter-102.html --json --stream-url

"url_manifest": null,


might return the type of the manifest or use a different name, not url_manifest


the output seems weird sometimes

@bastimeyer
Copy link
Member Author

--json --stream-url

These params are mutually exclusive...

might return the type of the manifest or use a different name, not url_manifest

And as said, it's only a problem if the plugin yields streams from different sources, but I don't see any way of fixing this in the regular --stream-url output. If the user hasn't selected a stream/quality, and the plugin yields different stream types, what should be done here other than selecting the last (usually the "best"/"best-unfiltered") stream? Reading the value of all streams and returning everything is not possible, because --stream-url is supposed to return only a single URL. Returning just one, sorted by the number of occurrences seems confusing if the number of stream types varies.

Regarding the name, the problem is that streamlink_cli should not have individual implementations for the various stream types, so a generic name needs to be chosen. I went with "manifest", because that works with DASH streams as well as HLS streams, and on HLS streams, "master" or "variant" are both specific terms only related to it. Not sure about the other stream types.

In the JSON output, as opposed to being null, the url_manifest property could be removed if a value doesn't exist, if you prefer it that way. That could prevent some kind of confusion, but a parser would have to deal with conditional properties. Or as a better solution, the "manifest-url" could be added to the object (and string representation) of each stream item. That would make it less legible with lots of repetitions, but would solve the stream type issue.

the output seems weird sometimes

The output hasn't been changed at all.

@bastimeyer bastimeyer force-pushed the feature/stream-url-manifest branch from 6006bef to 1bcf529 Compare October 28, 2020 20:06
@bastimeyer
Copy link
Member Author

Removed the url_manifest property from --json again since each stream item has its own JSON representation and HLS streams with a master playlist now have an additional master property.

Also changed the string representation of the HLSStream class to include the master playlist URL if it exists, and renamed the internal property and constructor parameter to url_master.

The to_url_manifest interface still exists on the abstract Stream class, as I couldn't find a better name that fits into every stream type. This method is needed for the --stream-url parameter, which reads the value from the last stream item in the resolved streams dict, which is usually "best".

Copy link
Member

@gravyboat gravyboat left a comment

Choose a reason for hiding this comment

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

Looks good, if @back-to has no further objections let's get it merged.

@beardypig
Copy link
Member

I'd think to_manifest_url is a better name, as it's the url to the manifest, not the manifest of the url?

@bastimeyer
Copy link
Member Author

@beardypig My idea was to have "manifest" as a suffix, so that it fits into the scheme of the existing interface. I can change it though to your suggestion which makes more sense readability-wise.

@bastimeyer
Copy link
Member Author

What's the decision here before I rebase? Rename it or not?

@gravyboat
Copy link
Member

@bastimeyer I'm not particularly passionate about either option, let's just rename it to to_manifest_url so we can get this merged.

This enables the output of the input-stream's manifest/master URL via
`streamlink --stream-url URL` / `streamlink --json URL`
in addition to the already supported output when selecting a stream via
`streamlink --stream-url URL STREAM` / `streamlink --json URL STREAM`

This is primarily helpful for HLS streams with a variant playlist.
@bastimeyer bastimeyer force-pushed the feature/stream-url-manifest branch from 1bcf529 to 7616a8e Compare October 31, 2020 20:46
@back-to back-to merged commit 9384f3d into streamlink:master Nov 1, 2020
@bastimeyer bastimeyer deleted the feature/stream-url-manifest branch January 19, 2021 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retreiving master HLS playlist/manifest of streams
4 participants