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

Add subtitles FPS label on found subs list #4763

Open
browser-bug opened this issue Jan 8, 2024 · 6 comments · Fixed by #4771
Open

Add subtitles FPS label on found subs list #4763

browser-bug opened this issue Jan 8, 2024 · 6 comments · Fixed by #4771

Comments

@browser-bug
Copy link

browser-bug commented Jan 8, 2024

What you want IINA to do:
When displaying the subtitles found (using Opensubtitles.com, etc.) show also the FPS of the subs if available:
image

What IINA does currently:
It shows the subs list without this information. In particular it currently only shows:

  • Language
  • Download number
  • Stars
  • Upload date

Why you think this should be added:
This would benefit the choice of the sub since it depends on the video source FPS. It is quite common that multiple subtitles are found but I cannot choose the correct one and therefore find myself going to the site for a double check (which makes the in-player download feature almost useless).

@low-batt low-batt self-assigned this Jan 11, 2024
low-batt added a commit that referenced this issue Jan 11, 2024
This commit will:
- Change the Subtitle struct in OpenSubClient to always include the fps
  property
- Change the getDescription method in OpenSub to include the fps in the
  subtitle chooser if Open Subtitles provided a value
@low-batt low-batt linked a pull request Jan 11, 2024 that will close this issue
2 tasks
@low-batt
Copy link
Contributor

@browser-bug Thanks for entering this issue.

As the spacing is rather tight I rounded the fps up. Do you think that is sufficient or is more precision required?
subtitle-fps

@browser-bug
Copy link
Author

browser-bug commented Jan 12, 2024

@low-batt
Thanks to you for addressing the issue. I guess a round up will do aswell, since the most common values are always 24, 25, 30 or 60 to my knowledge. But still, being more precise doesn't hurt. If the spacing is too tight and there are no other options I guess rounding will do just fine. Maybe an icon on the left?

@low-batt
Copy link
Contributor

It is looking like our next release will be focused on fixing regressions with a feature release to follow. To get this in now for the next release I wanted to minimize the changes required. That is why I wanted to avoid trying to expand the size of this UI. I do agree that more precision is desirable.

The current code for downloading subtitles is considered legacy code. The plan is to replace the existing code with a plugin implementation. Hopefully that work will include enhancements to the user interface.

On using an icon, I did look around for an icon. I found a lot of them. They were associated with online gaming. I didn't find anything I thought people would recognize as frames per second. Are you aware of something?

For this change I would not have used an icon as that would require a lot of changes as the UI is not structured for that. What looks like icons in the UI is actually characters:

⬇︎
DOWNWARDS BLACK ARROW
Unicode: U+2B07 U+FE0E, UTF-8: E2 AC 87 EF B8 8E


BLACK STAR
Unicode: U+2605, UTF-8: E2 98 85

I will check with the other developers to see if they think it is ok to squeeze in a couple more characters.

uiryuu pushed a commit that referenced this issue Jan 19, 2024
This commit will:
- Change the Subtitle struct in OpenSubClient to always include the fps
  property
- Change the getDescription method in OpenSub to include the fps in the
  subtitle chooser if Open Subtitles provided a value
@low-batt
Copy link
Contributor

The fix for this issue has been merged into the IINA develop branch. GitHub automatically closed the linked issue in reaction to the merge. I am reopening this issue until the fix is available in an official release of IINA so that users intending to report this problem can easily locate this existing report. Once the fix for this issue has been included in an official release this issue will be closed.

@low-batt low-batt reopened this Jan 20, 2024
@rasitayaz
Copy link

rasitayaz commented Jan 25, 2024

I think a more compact date & time formatting would save some space for the other items. I doubt anyone will ever want to know the exact seconds of the upload date.

@rasitayaz
Copy link

rasitayaz commented Jan 25, 2024

also related to this menu, I'd like to bring up #4142. having the subtitle file name multilined or marquee-like would be great.

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.

4 participants