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

[puhutv] Add new extractor #16269

Closed
wants to merge 18 commits into from
Closed

Conversation

mrfade
Copy link
Contributor

@mrfade mrfade commented Apr 24, 2018

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

New extractor added due to issue#16010


return {
'id': id,
'slug_id': video_id,
Copy link
Collaborator

Choose a reason for hiding this comment

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

No such meta field.

'description': info.get('title').get('description'),
'uploader': info.get('title').get('producer').get('name'),
'view_count': info.get('content').get('watch_count'),
'follower_count': info.get('title').get('follower_count'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

No such meta field.

'follower_count': info.get('title').get('follower_count'),
'thumbnail': 'https://%s' % info.get('content').get('images').get('wide').get('main'),
'thumbnails': thumbnails,
'loop_thumbnails': info.get('content').get('loop_thumbnails'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

No such meta field.

'uploader': info.get('title').get('producer').get('name'),
'view_count': info.get('content').get('watch_count'),
'follower_count': info.get('title').get('follower_count'),
'thumbnail': 'https://%s' % info.get('content').get('images').get('wide').get('main'),
Copy link
Collaborator

@dstftw dstftw Apr 24, 2018

Choose a reason for hiding this comment

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

Read coding conventions on optional metadata and fix code appropriately.

@dstftw dstftw mentioned this pull request Apr 24, 2018
8 tasks
@mrfade mrfade closed this Apr 24, 2018
@mrfade mrfade deleted the puhutv-new-extractor branch April 24, 2018 19:09
@mrfade mrfade restored the puhutv-new-extractor branch April 24, 2018 19:09
@mrfade mrfade reopened this Apr 24, 2018
@mrfade
Copy link
Contributor Author

mrfade commented Apr 30, 2018

I have finished all commits, Could you review again? @dstftw

@dstftw
Copy link
Collaborator

dstftw commented Apr 30, 2018

Optional meta fields - nothing changed.
flake8 - not checked.

@mrfade
Copy link
Contributor Author

mrfade commented Jun 6, 2018

@dstftw Could you review again? If there is any problem, could you leave feedback


display_id = compat_str(info['id'])
title = info['title']['name']
if(info.get('display_name') and title is not None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Outer parenthesis are superfluous. title can't be None at this point.

if(info.get('display_name') and title is not None):
title += ' ' + info.get('display_name')

description = info.get('title', {}).get('description') or info.get('description')
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.

title += ' ' + info.get('display_name')

description = info.get('title', {}).get('description') or info.get('description')
timestamp = parse_iso8601(info.get('created_at'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

unified_timestamp.

description = info.get('title', {}).get('description') or info.get('description')
timestamp = parse_iso8601(info.get('created_at'))
upload_date = unified_strdate(info.get('created_at'))
uploader = info.get('title', {}).get('producer', {}).get('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. Same for all other places.

uploader_id = str_or_none(info.get('title', {}).get('producer', {}).get('id'))
view_count = int_or_none(info.get('content', {}).get('watch_count'))
duration = float_or_none(info.get('content', {}).get('duration_in_ms'), scale=1000)
thumbnail = urljoin('https://', info.get('content', {}).get('images', {}).get('wide', {}).get('main'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

First parameter to urljoin is URL.

'https://puhutv.com/api/assets/%s/videos' % display_id,
video_id, 'Downloading video JSON', fatal=False)
if not req_formats:
self.raise_geo_restricted()
Copy link
Collaborator

Choose a reason for hiding this comment

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

It does not necessarily mean that.

pagenum = 1
has_more = True
while has_more is True:
query = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inline to actual call place.

continue
subtitles[self._SUBTITLE_LANGS.get(lang, lang)] = [{
'url': sub_url,
'ext': determine_ext(sub_url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for ext.

@mrfade
Copy link
Contributor Author

mrfade commented Jun 11, 2018

I fixed it as you requested. Would you like to have a look? @dstftw @remitamine @rg3

@dstftw dstftw closed this in 8fd2a7b Jul 22, 2018
dstftw added a commit that referenced this pull request Jul 22, 2018
@mrfade mrfade deleted the puhutv-new-extractor branch July 22, 2018 15:00
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.

2 participants