-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
plugins.artetv: only pick the first variant of the stream #3228
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
Conversation
There are still potentially up to 4 additional alternative HLS streams for each stream input which would override each other's stream selections. Just like I've described in #3167 (comment). If there really is a need for those alt-streams other than the main one, then we should add a plugin parameter for that, which selects those alt-streams. |
Please check the commit message prefix. There's a typo in the plugin name. |
c5c66ad
to
ebba262
Compare
This comment has been minimized.
This comment has been minimized.
I'm not sure which ones would come up as
A plugin parameter could be added to select which stream you wanted, you'd also need an option to list them. Regardless, that should be a separate PR as this is intended to fix the immediate issue highlighted in #3167. |
I think renaming the yielded streams could just work. Add the code as prefix with lowercase text and underscores instead of dashes. There will be a lot more streams to select, but at least they don't override each other. A plugin parameter would then not be needed. |
After closer inspection I see there is a much better way to do it by selecting the default variant, I will also prepare a PR with a variant selection option that can be review separately, I think it makes more sense than listing 20 streams. |
ebba262
to
93c01af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked the API responses again and it looks like this is a good enough solution for now.
@beardypig, if you want to make one little refinement, then please move the log = logger.getLogger(__name__)
definition up, underneath the imports, and also change the log.error
call so that the logger receives the already formatted message string (this way, it can get translated into an f-string later on automatically), thanks.
93c01af
to
f8448cc
Compare
25ee167
to
cdcea57
Compare
src/streamlink/plugins/artetv.py
Outdated
def _create_stream(self, streams): | ||
variant, variantname = min([(stream["versionProg"], stream["versionLibelle"]) for stream in streams.values()], | ||
key=itemgetter(0)) | ||
log.debug("Using the '{0}' stream variant".format(variantname)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error with py2 because of Français
$ streamlink https://www.arte.tv/fr/direct/ -l info
src/streamlink_cli/main.py:760: PythonDeprecatedWarning: Python 2.7 ...
[cli][info] Found matching plugin artetv for URL https://www.arte.tv/fr/direct/
error: 'ascii' codec can't encode character u'\xe7' in position 11: ordinal not in range(128)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot. This should fix it.
log.debug("Using the '{0}' stream variant".format(variantname)) | |
log.debug(u"Using the '{0}' stream variant".format(variantname)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it on the error log too.
cdcea57
to
68a701a
Compare
Nice, now selecting the |
@quantenschaum as a compromise there is only the primary language stream available right now, to change the language we need an extra option. |
Yes, please do so. The option |
Removes the f4m and http streams from the returned list, and only returns the default variant for the stream. I believe this will match the expected behaviour.
see #3167