-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Close download from Open Subtitles using REST API, #3920 #4678
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
Conversation
This commit will: - Add a new OpenSubClient class - Change the OpenSub class to use the new client - Change the OpenSub default language from eng to en - Change name of provider from opensubtitles.org to opensubtitles.com - Add a new loggedIn property to OnlineSubtitle - Add a new logout method to OnlineSubtitle - Add a new iinaLogoutCompleted notification to AppData - Change AppDelegate to log out of the online subtitle provider during termination This is a temporary stopgap solution that updates the builtin legacy code in order to meet the end of 2023 deadline imposed by Open Subtitles for switching to the new REST API. The plan is still to replace all built-in code for downloading from online subtitle providers with plug-in implementations.
Preferred ImplementationThe plan is to replace all built-in code for downloading from online subtitle providers with plug-in implementations. Thus the preferred way to address the need to switch from the Open Subtitles XLMRPC API to their new REST API is to do so as a part of the refactoring of the code into a plug-in. Potential ProblemThe potential problem with this plan is due to the following from OpenSubtitles.org API deprecation and end of life:
Assuming:
Then this would require the IINA 1.4.0 release to be out before the end of the year. Possibly doable, but awfully aggressive based on the time taken for previous releases. There are quite a few tasks listed in the 1.4.0 goals. This does not seem like a release that should be rushed. Temporary STOPGAPThe commit in this PR does not contain the desired implementation. That requires a JavaScript expert. This PR is the backup plan and contains a temporary stopgap solution that updates the builtin legacy code. If we can come up with a way to implement and release a plug-in version before the end of year deadline then there will be no need to review this PR and it can be directly tossed into the trash. This PR provides an option for an IINA 1.3.4 release that addresses the 1.3.3 regressions and ensures continued download support from Open Subtitles. Considerations for Plug-in ImplementationAnother reason to not rush the refactoring of the built-in code into plug-ins is that existing issues that should be addressed when the code is rewritten. I suspect some of these issues will be easier to address with plug-ins. The ones I can remember:
With Open Subtitles it is especially important to clearly report the status of a user's download quota. The Open Subtitles REST API provides lots of options that can be specified when searching. It would be good to fully support these features. Search results now support paging. There are lots of details returned about each subtitle file that users might want to see. As this PR is only a temporary stopgap it does not address any of these problems. DraftThis PR is still a draft is because IINA needs to obtain an Open Subtitles API key. The code has been checked in without an API key. Look for "FIXME" in Release NotesUsers with an opensubtitles.org account will be unable to download subtitles if they have not imported their accounts into opensubtitles.com. Not sure if Open Subtitles users are aware of this change. This definitely needs to be called out in the release notes. Maybe something like the following: Open Subtitles opensubtitles.org is being rebuilt as opensubtitles.com. API access for downloading subtitles from opensubtitles.org will be shutdown at the end of 2023. This version of IINA has been changed to use opensubtitles.com instead of opensubtitles.org. Users with opensubtitles.org accounts must import their accounts into opensubtitles.com. See issue #3920 for details. LoggingThe lack of logging in this area of IINA has hindered diagnosing problems downloading subtitles. Adding to this problem is that the responses from Open Subtitles are not consistent. Open Subtitles will return HTML error pages, apparently due to overload. By the time a developer tries to reproduce the problem everying is working fine. These changes include extensive logging as can be seen in this example: |
This commit will: - Add a new OpenSubClient class - Change the OpenSub class to use the new client - Change the OpenSub default language from eng to en - Change name of provider from opensubtitles.org to opensubtitles.com - Add a new loggedIn property to OnlineSubtitle - Add a new logout method to OnlineSubtitle - Add a new iinaLogoutCompleted notification to AppData - Change AppDelegate to log out of the online subtitle provider during termination This is a temporary stopgap solution that updates the builtin legacy code in order to meet the end of 2023 deadline imposed by Open Subtitles for switching to the new REST API. The plan is still to replace all built-in code for downloading from online subtitle providers with plug-in implementations.
aea2c32
to
5b066ee
Compare
It doesn't seem as precise as the older version. There are many video files which are not finding anything through opensubtitles.com. Is this a limitation to the new REST Api or did you change some parameters during the call? I cannot find the exact implementation but are we still searching by moviehash and/or by name as (I guess) we did before? |
Lets continue this discussion in the new issue #4749.
The particular problem #4749 identifies has to do with results for TV
episodes not being narrowed to the specific episode. Possibly that is
the problem behind the lack of precision being reported?
The new REST API is quite different from the old API. It still uses hash
codes and the new code is passing the hash code to Open Subtitles.
In investigating #4749 I found an Open Subtitles utility API that
attempts to extract information from the filename. Apparently the new
API expects clients to call this and add the extracted information to
the search query. I am currently finishing code that does this and it
looks like that will address the problem with TV episodes.
The report of not finding anything is concerning. To investigate that I
need a specific example that shows this.
Communication with Open Subtitles is now fully logged. With a log file I
can tell you exactly what happened.
The 1.3.4 release was rushed to meet the Open Subtitles deadline. Quite
a few tasks were left undone. We are planning issuing another release
soon that addresses those issues along with the above mentioned change
to the Open Subtitles support.
Lets continue this discussion in #4749. If you can provide an example
where no results are returned I will investigate that ASAP.
…On 1/6/24 5:08 AM, Bernardo Menicagli wrote:
It doesn't seem as precise as the older version. There are many video files which are not finding anything through [opensubtitles.com](). Is this a limitation to the new REST Api or did you change some parameters during the call? I cannot find the exact implementation but are we searching by moviehash and/or by name?
|
This commit will:
OpenSubClient
classOpenSub
class to use the new clientOpenSub
default language fromeng
toen
opensubtitles.org
toopensubtitles.com
loggedIn
property toOnlineSubtitle
logout
method toOnlineSubtitle
iinaLogoutCompleted
notification toAppData
AppDelegate
to log out of the online subtitle provider during terminationThis is a temporary stopgap solution that updates the builtin legacy code in order to meet the end of 2023 deadline imposed by Open Subtitles for switching to the new REST API. The plan is still to replace all built-in code for downloading from online subtitle providers with plug-in implementations.
Description: