Add Cinemast provider #817

ofir123 · 2017-10-02T22:43:50Z

Fixed SubsCenter provider to work with the new API.

fernandog

Provider should be called cinemast now right?

fernandog · 2017-11-17T12:24:06Z

@ofir123 also http://www.subscenter.info/he/ still exists

ofir123 · 2017-11-18T17:43:00Z

This provider is based on the SubsCenter Kodi plugin.
I think they're the same (Cinemast is just an alternative name).. but of course I'll rename it if it's a problem.
subscenter still exists, but they added a CAPTCHA so automatic downloads are problematic.

fernandog · 2017-11-25T17:27:19Z

@ofir123 ok but if site is Cinemast I would rename to not confuse users

ofir123 · 2017-11-29T20:57:09Z

Renamed to Cinemast!

fernandog · 2017-11-29T20:59:25Z

@ofir123 can you merge develop branch into this branch? Or rebase on develop

ofir123 · 2017-11-29T21:17:21Z

Sure, rebased now.

fernandog · 2017-11-29T21:22:38Z

When you rebase only your commits should appear

This reverts commit bb72095.

fernandog · 2017-11-29T21:38:04Z

@ofir123 do this:

git checkout origin/develop
git pull
git checkout feature/new-subscenter-support
git rebase develop feature/new-subscenter-support
git push feature/new-subscenter-support -f

ofir123 · 2017-11-29T21:47:04Z

Oh sorry about that :
Wrong direction.. fixed it now (and thanks for the tips @fernandog )

fernandog · 2017-11-29T22:53:09Z

@ofir123 still has duplicate commits. I will try to fix tomorrow

fernandog · 2017-11-29T23:29:54Z

@ofir123 fixed

Already renamed

fernandog · 2017-11-29T23:41:44Z

@ofir123 can you add a test like legendastv that would only match a title if using an alternative title? Do you know one case that only return results when searching with alternative title?

Only movies has `title` as match

ofir123 · 2017-11-30T21:03:59Z

@fernandog thnx for the fixes!
I've added the test you requested..

fernandog · 2017-12-01T00:14:07Z

@ofir123 all tests passing

@Diaoul this is the the subscenter provider (which is in master) that got renamed to Cinemast.
Good to merge?

Diaoul · 2017-12-01T08:09:15Z

subliminal/providers/cinemast.py

+
+        self.session = None
+        self.username = username or self.default_username
+        self.password = password or self.default_password


Why is there a default user?

API is accessible only to registered users.

In that case, don't provide a default user, just use this one for testing and require USER and PASSWORD to be provided in the CLI.
I don't think we have private-only providers so this is all new for you to do.

I really don't think this is necessary for this provider, since it's not really private.
Only the API requires user and password, and the actual website doesn't.
They don't really care about it being a default user as far as I can tell..

If/When they introduce a limited number of downloads per user this is not going to work anymore. Moreover this is not a widely used providers as per the only language it supports. I don't think people would mind entering a user/password in their command line.
Unless you have explicit agreement with Cinemast to use a default user I'd rather require subliminal users to register.

A websites generates ad-revenue, an API does not. does not so maybe api restriction is intentional.

OK, removed.

Diaoul · 2017-12-01T08:11:05Z

subliminal/providers/cinemast.py

+        self.session.headers['User-Agent'] = 'Subliminal/{}'.format(__short_version__)
+
+        # login
+        if self.username and self.password:


This will always evaluate to True with default values.

Verification is for user input (in case None or an '' were inserted).

@ofir123
do like legendastv:

if self.username is not None and self.password is not None:

self.username = username or self.default_username this already takes care of that.

Diaoul · 2017-12-01T08:14:13Z

subliminal/providers/cinemast.py

+        subtitles = {}
+        for group_data in results.get('data', []):
+            # create page link
+            slug_name = group_data.get('name_en').lower().replace(' ', '-').replace('\'', '').replace('"', '')


What about other characters? e.g. dot, semicolons
Is there any other way to get this information?

Not really..
It's not my site so I'm just guessing and checking how to replace each character.
I'll added dot and semicolons though! Good idea.

Make this a function so it doesn't look too long.

Diaoul · 2017-12-01T08:14:45Z

subliminal/providers/cinemast.py

+            # create page link
+            slug_name = group_data.get('name_en').lower().replace(' ', '-').replace('\'', '').replace('"', '')
+            if query['type'] == 'series':
+                page_link = self.server_url + 'subtitle/series/{}/{}/{}/'.format(slug_name, season, episode)


urlencode is required here due to above comment.

@Diaoul urlencode is from urllib right? Requests already does the encoding and it's not needed. confirm?

no other provider uses urlencode

For params or data you are correct but even for the URL requests does that? Anyway, I'm not sure this makes any sense as with special characters the URL will probably be wrong no? If you can add tests that pass with accentuated characters and weird characters that'll be fine for me.

Diaoul · 2017-12-01T08:14:56Z

subliminal/providers/cinemast.py

+            if query['type'] == 'series':
+                page_link = self.server_url + 'subtitle/series/{}/{}/{}/'.format(slug_name, season, episode)
+            else:
+                page_link = self.server_url + 'subtitle/movie/{}/'.format(slug_name)


Diaoul · 2017-12-01T08:18:55Z

subliminal/providers/cinemast.py

+        else:
+            titles = [video.title] + video.alternative_titles
+
+        for title in titles:


As I said before, this is going to flood the servers with many useless requests sometimes returning duplicated results. I'm really not a big fan of this.

@Diaoul It will stop in the first result found. And dogpile caches it so next time it won't hit server.

Diaoul · 2017-12-01T08:20:19Z

subliminal/providers/cinemast.py

+        # download
+        url = self.server_url + 'subtitle/download/{}/'.format(subtitle.language.alpha2)
+        params = {
+            'v': subtitle.releases[0],


Does the result change if you take another release?

Diaoul · 2017-12-01T08:22:11Z

subliminal/providers/cinemast.py

+                matches.add('episode')
+            # guess
+            for release in self.releases:
+                matches |= guess_matches(video, guessit(release, {'type': 'episode'}))


This is potentially going to give an unreal good score to the subtitles as this is the union of all releases instead of picking the best.

Not sure I understand what you mean..
If the subtitle matches multiple releases, I want to check all of them.
Should I change it so each subtitle will match only one release?

I don't know in that case if we shouldn't do a single Subtitle instance for each release. Even if this is the same actual subtitle.
I think for now if it does the job leave it as is but that's a thought for the long term.
@pannal: what about harmonizing the Subtitle object so that it's not provider specific but rather "subliminal"-specific? This would avoid you patching all the subtitle classes to change the guessing rules. Not sure the feasability though.

Diaoul · 2017-12-01T08:22:18Z

subliminal/providers/cinemast.py

+        elif isinstance(video, Movie):
+            # guess
+            for release in self.releases:
+                matches |= guess_matches(video, guessit(release, {'type': 'movie'}))


Diaoul · 2017-12-01T08:22:45Z

subliminal/providers/cinemast.py

+
+                    # otherwise create it
+                    subtitle = self.subtitle_class(language, page_link, title, season, episode, title, subtitle_id,
+                                                   subtitle_key, [release])


1 release = 1 instance

ofir123 · 2017-12-01T12:26:40Z

OK, fixed multiple releases issue. @Diaoul

ofir123 · 2017-12-01T21:57:09Z

@fernandog How do you fix the tests? Is there something I'm doing wrong with the cassettes?

fernandog · 2017-12-02T21:05:30Z

all good now

ofir123 · 2017-12-05T21:39:01Z

@Diaoul @fernandog So what's missing for the merge?

smoresmores · 2018-05-07T17:17:46Z

What is the status of this?
@ofir123 @fernandog @Diaoul

ofir123 · 2018-05-08T20:59:35Z

@smoresmores It's ready.. just waiting for the merge..

# Conflicts: # setup.py # subliminal/cli.py # subliminal/extensions.py # tests/test_extensions.py

ofir123 · 2019-04-25T17:09:01Z

Hey @ratoaq2 !
Any chance this will get merged in the upcoming version?

Because Travis Ci is on vacation.

fernandog previously requested changes Nov 17, 2017

View reviewed changes

ofir123 changed the title ~~Feature/new subscenter support~~ Feature/new cinemast provider Nov 29, 2017

ofir123 added 4 commits November 29, 2017 23:37

Revert "Removed SubsCenter provider."

26885b3

This reverts commit bb72095.

Fixed subscenter provider!

07fc287

Changed provider name to cinemast.

d7d1769

Fixed sorting everywhere.

5d74d55

ofir123 force-pushed the feature/new-subscenter-support branch from 25b6d93 to 2bf87ef Compare November 29, 2017 21:45

fernandog force-pushed the feature/new-subscenter-support branch from 2bf87ef to 5d74d55 Compare November 29, 2017 23:29

Added aliases to the video

2dfc825

fernandog changed the title ~~Feature/new cinemast provider~~ Add Cinemast provider Nov 29, 2017

fernandog added the type/new-provider label Nov 29, 2017

fernandog and others added 3 commits November 29, 2017 21:58

Requests doesn't need urlencode

31464b6

New tests after urlencode + fix code and test for alternative titles

bc397d8

Only movies has `title` as match

Added alternative title test.

ddee12f

Fix tests

ecd885d

fernandog force-pushed the feature/new-subscenter-support branch from 90063b5 to ecd885d Compare November 30, 2017 22:13

Diaoul requested changes Dec 1, 2017

View reviewed changes

Fixed multiple releases issue.

2ca23cf

ofir123 added 6 commits December 1, 2017 14:28

Fixed login check.

7f55a9d

Removed default username and password.

59a1312

Updated cassettes.

f5d799d

Tiny parameters fix.

f26b08f

Fixed core tests.

b634a52

New cassettes.

6eb24c1

Fix tests

1e75c43

natiz pushed a commit to natiz/Sub-Zero.bundle that referenced this pull request Jan 25, 2018

make subscenter based on a PR from ofir123 - Diaoul/subliminal#817

f783758

vledder mentioned this pull request Apr 5, 2018

Adding more search providers from python project Sub.Zero caronc/nzb-subliminal#51

Open

ratoaq2 self-assigned this Feb 24, 2019

ratoaq2 added this to the 2.1 milestone Feb 24, 2019

ofir123 added 2 commits April 25, 2019 20:01

Merge branch 'develop' into feature/new-subscenter-support

be3cb67

# Conflicts: # setup.py # subliminal/cli.py # subliminal/extensions.py # tests/test_extensions.py

Fixed tests.

11af13a

ofir123 and others added 3 commits May 8, 2019 12:29

Updated cassettes.

29d3358

GitHub Action to lint Python code

e25589d

Because Travis Ci is on vacation.

Merge branch 'master' into feature/new-subscenter-support

e140104

Add Cinemast provider #817

Are you sure you want to change the base?

Add Cinemast provider #817

Conversation

ofir123 commented Oct 2, 2017

fernandog left a comment

Choose a reason for hiding this comment

fernandog commented Nov 17, 2017

ofir123 commented Nov 18, 2017

fernandog commented Nov 25, 2017

ofir123 commented Nov 29, 2017

fernandog commented Nov 29, 2017

ofir123 commented Nov 29, 2017

fernandog commented Nov 29, 2017

fernandog commented Nov 29, 2017

ofir123 commented Nov 29, 2017

fernandog commented Nov 29, 2017

fernandog commented Nov 29, 2017

fernandog commented Nov 29, 2017 • edited Loading

ofir123 commented Nov 30, 2017

fernandog commented Dec 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Diaoul Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofir123 Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

fernandog Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fernandog Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofir123 Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofir123 commented Dec 1, 2017 • edited Loading

ofir123 commented Dec 1, 2017

fernandog commented Dec 2, 2017

ofir123 commented Dec 5, 2017

smoresmores commented May 7, 2018 • edited Loading

ofir123 commented May 8, 2018

ofir123 commented Apr 25, 2019

fernandog commented Nov 29, 2017 •

edited

Loading

Diaoul Dec 1, 2017 •

edited

Loading

ofir123 Dec 1, 2017 •

edited

Loading

fernandog Dec 1, 2017 •

edited

Loading

fernandog Dec 1, 2017 •

edited

Loading

ofir123 Dec 1, 2017 •

edited

Loading

ofir123 commented Dec 1, 2017 •

edited

Loading

smoresmores commented May 7, 2018 •

edited

Loading