Skip to content

Conversation

@beardypig
Copy link
Member

@beardypig beardypig commented Oct 6, 2020

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

@bastimeyer bastimeyer added the plugin issue A Plugin does not work correctly label Oct 7, 2020
@bastimeyer
Copy link
Member

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.

@bastimeyer bastimeyer changed the title plugins.atretv: simplify the list of streams returned plugins.artetv: simplify the list of streams returned Oct 7, 2020
@bastimeyer
Copy link
Member

Please check the commit message prefix. There's a typo in the plugin name.

@beardypig beardypig force-pushed the arte-simplified branch 3 times, most recently from c5c66ad to ebba262 Compare October 9, 2020 11:41
@codecov

This comment has been minimized.

@beardypig
Copy link
Member Author

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

I'm not sure which ones would come up as alt, I believe the list of codes that are filtered out are only the regular audio tracks. If you could provide a counter case that would be helpful.

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.

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.

@bastimeyer
Copy link
Member

I'm not sure which ones would come up as alt

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.

@beardypig
Copy link
Member Author

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.

@beardypig beardypig changed the title plugins.artetv: simplify the list of streams returned plugins.artetv: only pick the first variant of the stream Oct 9, 2020
Copy link
Member

@bastimeyer bastimeyer left a 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.

@beardypig beardypig force-pushed the arte-simplified branch 2 times, most recently from 25ee167 to cdcea57 Compare October 16, 2020 09:33
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))
Copy link
Collaborator

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)

Copy link
Member Author

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.

Suggested change
log.debug("Using the '{0}' stream variant".format(variantname))
log.debug(u"Using the '{0}' stream variant".format(variantname))

Copy link
Member Author

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.

@back-to back-to added the plugin enhancement A new feature for a working Plugin label Oct 17, 2020
@back-to back-to merged commit a23c3e3 into streamlink:master Oct 17, 2020
@quantenschaum
Copy link

Nice, now selecting the best stream consistently returns the stream with the "right" language. But what, if I want to manually select another stream (different language/subtitles)? The 720_alt or 720p_alt2 are no longer available now.

@beardypig
Copy link
Member Author

@quantenschaum as a compromise there is only the primary language stream available right now, to change the language we need an extra option.

@quantenschaum
Copy link

Yes, please do so. The option --hls-audio-select does not have an effect on the arte plugin. How can I download the stream in a different language? Maybe by changing the URL to point the page page in a different language? Is the order of streams different between the different language versions of the page?

resiproxy pushed a commit to resiproxy/streamlink that referenced this pull request Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin enhancement A new feature for a working Plugin plugin issue A Plugin does not work correctly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants