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

[skillshare:course] Add new extractor #16582

Closed
wants to merge 3 commits into from
Closed

Conversation

Oshawk
Copy link

@Oshawk Oshawk commented May 30, 2018

Added new extractor for skillshare.com classes (you can't link to a specific video).

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

Added support for skillshare.com classes such as this using APIs calls extracted from the mobile app.
Tested with both premium and free courses and premium and free users. Works like any other extractor, requires username and password.
Individual video support not added as I can find no way of producing a URL for a specific video.

Added new extractor for skillshare.com classes (you can't link to a specific video).
@Oshawk Oshawk mentioned this pull request May 30, 2018
4 tasks


class SkillshareBaseIE(InfoExtractor):
_NETRC_MACHINE = 'udemy'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrong id.

class SkillshareBaseIE(InfoExtractor):
_NETRC_MACHINE = 'udemy'

_TN_RE = r"uploads/video/thumbnails/[0-9a-f]+/(?P<width>[0-9]+)-(?P<height>[0-9]+)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Single quotes.

Copy link
Author

@Oshawk Oshawk Jun 1, 2018

Choose a reason for hiding this comment

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

Should everything be single quotes? @dstftw

Copy link
Author

Choose a reason for hiding this comment

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

Oh, you mean make double?

def _login(self):
username, password = self._get_login_info()
if username is None or password is None:
self.raise_login_required("An email and password is needed to download any video (even non-premium ones)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Must allow cookie authentication.

Copy link
Author

Choose a reason for hiding this comment

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

@dstftw I have done some testing and this is not possible, web cookies don't seem to translate to mobile cookies.

from ..utils import int_or_none


class SkillshareBaseIE(InfoExtractor):
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no point in base class.

Copy link
Author

Choose a reason for hiding this comment

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

My thought on the base class was to make it simpler to allow more features such single video / playlist downloading later on. @dstftw

class_id = self._match_id(url)
class_json = self._download_json(self._CLASS_URL % class_id,
None,
note="Getting class details",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Downloading class JSON.


videos.append({
"id": str(lesson_json["id"]),
"title": lesson_json.get("title"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Title is mandatory.

"uploader": class_json["_embedded"].get("teacher", {}).get("full_name"),
"creator": class_json["_embedded"].get("teacher", {}).get("full_name"),
"timestamp": lesson_timestamp,
"uploader_id": str(class_json["_embedded"].get("teacher", {}).get("username", 0)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lack of data is denoted by None not int.

"ext": "mp4",
"thumbnails": lesson_thumbnails_json,
"uploader": class_json["_embedded"].get("teacher", {}).get("full_name"),
"creator": class_json["_embedded"].get("teacher", {}).get("full_name"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

try_get.

"creator": class_json["_embedded"].get("teacher", {}).get("full_name"),
"timestamp": lesson_timestamp,
"uploader_id": str(class_json["_embedded"].get("teacher", {}).get("username", 0)),
"categories": [class_json.get("category")],
Copy link
Collaborator

Choose a reason for hiding this comment

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

No [None] should be possible.

videos = []
for lesson_json in lessons_json:
lesson_thumbnail_urls = [
lesson_json.get("video_thumbnail_url", ""),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Processing empty strings is senseless.

Fixed all noted issued except cookie authentication (desktop and mobile don't correlate) and base class (to allow more functionality to be added in the future).
Improved JSON download message.
@Oshawk
Copy link
Author

Oshawk commented Oct 4, 2018

Hi, this seems to have gone stagnant. @dstftw I have fixed everything except cookie authentication I think (which would require a completely new way of getting the videos).

@partopronto
Copy link

I have tested this pull request - works for paid and free content.
My only suggestion please add: user_type "trial"
i.e.:
...
elif user_type == "Premium Member" or "trial":
...

"Trial Membership" is essentialy a "Premium Membership"

@bdrtsky
Copy link

bdrtsky commented Mar 27, 2019

Is it working?

@Ruthalas
Copy link

Ruthalas commented Apr 8, 2019

Can this be merged?

@Oshawk
Copy link
Author

Oshawk commented Oct 21, 2019

It seems like this will never get merged. Closing.

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.

None yet

5 participants