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

[YleAreena] Add new extractor #20274

Closed
wants to merge 3 commits into from
Closed

Conversation

Hegezcc
Copy link

@Hegezcc Hegezcc commented Mar 8, 2019

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

This pull request adds a new extractor for Yle Areena, Finnish Broadcasting Company internet service. There has been an unmerged extractor pull request for both Yle Areena and Yle Elävä Arkisto using the old Flash player technology, but that player has been completely phased out from Yle Areena and replaced with Kaltura streaming service. As the codebases of these services differ greatly now, this project's scope is limited to be in Yle Areena.

Ping @hlintala for making the old extractor. Maybe the Yle Elävä Arkisto extractor might be resurrected from your code.

@Hegezcc
Copy link
Author

Hegezcc commented Mar 8, 2019

Looks like Yle is using aggressive geo-blocking which prevents tests in CI from passing. This is a major problem, as I cannot provide a really big proxy service for every downloader and also believe that the extractor's main userbase is in Finland, which would make the proxy pretty pointless.

How can this pull request proceed?

@dstftw
Copy link
Collaborator

dstftw commented Mar 8, 2019

Read coding conventions.

Following the style guide a bit better
@Hegezcc
Copy link
Author

Hegezcc commented Mar 8, 2019

@dstftw could you elaborate more on what is wrong in this code regarding coding conventions?

@Hegezcc Hegezcc mentioned this pull request Mar 8, 2019
@Hegezcc
Copy link
Author

Hegezcc commented Mar 11, 2019

Bump.

@Hegezcc
Copy link
Author

Hegezcc commented May 7, 2019

Bump. Anything still required here?

@hlintala
Copy link
Contributor

hlintala commented Sep 16, 2019

Maybe code should look something like

player_url = url_or_none(self._og_search_property(['video:secure_url', 'video:url'], webpage))

instead of

try:
    player_url = url_or_none(
        self._og_search_property('video:secure_url', webpage)
    )
except RegexNotFoundError:
        player_url = None

# If this backup fails extractor will error out
player_url = url_or_none(
    self._og_search_property('video:url', webpage)
)

Also trailing parenthesis probably should be located after last argument and not on a new line.

@subirax
Copy link

subirax commented Dec 27, 2020

Bump. Thanks!

@tpimh
Copy link

tpimh commented May 5, 2022

What's actually left to be done to merge this? I can provide a proxy service for CI to run if required.

@dirkf dirkf closed this Aug 1, 2023
@dirkf dirkf added the defunct PR source branch is not accessible label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defunct PR source branch is not accessible pending-fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants