8000 Failed to download subtitles from assrt due to rate limit. · Issue #4671 · iina/iina · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Failed to download subtitles from assrt due to rate limit. #4671

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

Closed
jjaychen1e opened this issue Oct 25, 2023 · 4 comments
Closed

Failed to download subtitles from assrt due to rate limit. #4671

jjaychen1e opened this issue Oct 25, 2023 · 4 comments

Comments

@jjaychen1e
Copy link

System and IINA version:

  • macOS 14.0
  • IINA 1.3.3 Build 138

Steps to reproduce:

  1. Use assrt as source of subtitles
  2. Choose a subtitle resource with more than 2 subtitles (which means there are more than 2 subtitles in the filelist, usually this happens when the resource contains subtitles for the whole season, rather than one episode)
  3. It should pop up a hint to display the download task is failed.

Cause analysis:

After validating the API and server ability of assrt, this issue is in the IINA side. When use proxy tools such as Proxyman, it's easy to guess the assrt server limits the concurrency requests.

How often does this happen?

Quite often

@jjaychen1e jjaychen1e changed the title Failed to downloads subtitles from assrt due to rate limit. Failed to download subtitles from assrt due to rate limit. Oct 25, 2023
@jjaychen1e
Copy link
Author

#4672 is not the perfect solution but it works well, because it would take seconds to download all subtitles. Maybe we could provide a window selector for users to pick files they want to download.

@low-batt
Copy link
Contributor

Hello from a junior IINA developer. Thank you for reporting this and especially for posting a pull request with a fix.

On this:

Maybe we could provide a window selector for users to pick files they want to download.

That is what SubChooseViewController is supposed to do? I'm only familiar with the code for Open Subtitles. In OpenSub a Subtitle instance is associated with one subtitle file. In Assrt the Subtitle class contains a fileList property. I know nothing about this site's API so I'm not understanding why there are multiple files. Maybe the same subtitles are available in different file formats?

In any case at this time a more important consideration is this code:

enum Origin {
case legacy
case plugin(id: String, name: String)
}

The built-in online subtitle provider code is now considered legacy code. This is because the plan is to switch to plug-ins for such implementation. The plug-in system is included in the current release of IINA, but is disabled as it is not quite ready to be officially released. The plan is to enable it in the upcoming feature release.

Given that, enhancements such as adding a window selector probably should be done as a part of the plug-in conversion.

Open Subtitles is shutting down their XMLRPC API at the end of 2023. I am currently finishing changing the current Open Subtitles legacy code to switch to their new REST API. This is a temporary stopgap solution that may end up going straight into the trash if a plugin implementation can be created in time to meet the deadline.

I'm thinking IINA should not rush the feature release. Given the limited time left in the year I plan on proposing that IINA release a maintenance release before starting to work on the feature release. I will add PR #4672 to the fixes that should be included in this release. Of course it will be up to the senior developers as to what plans are appropriate to adopt.

@low-batt
Copy link
Contributor

@lhc70000 has merged a tweaked version of the proposed fix into the develop branch.

@low-batt
Copy link
Contributor
low-batt commented Jan 1, 2024

This has been fixed in IINA 1.3.4.

Thanks @jjaychen1e for the fix!

@low-batt low-batt closed this as completed Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants
0