Skip to content

feat: hotkey for subtitle offset - #1067

Open
alex-rans wants to merge 2 commits into
DonutWare:developfrom
alex-rans:subtitle_offset_hotkeys
Open

feat: hotkey for subtitle offset#1067
alex-rans wants to merge 2 commits into
DonutWare:developfrom
alex-rans:subtitle_offset_hotkeys

Conversation

@alex-rans

Copy link
Copy Markdown

Pull Request Description

Adds hotkeys to offset subtitles forwards and backwards in time in increments of 250ms. The new hotkeys are left control + left/right arrow key. Adds an indicator to display how much the subtitles are offset in ms in the video player as well. The functionality appears to be somewhat buggy on the web client.

TODO?: Localization in video_player_settings.dart is currently just written in english, but I don't think it really matters that much as there's no way to display that text (as far as im aware)

Issue Being Fixed

Resolves #670

Screenshots / Recordings

Tested On

  • Android
  • Android TV
  • iOS
  • Linux
  • Windows
  • macOS
  • Web

Checklist

  • If a new package was added, did you ensure it works for all supported platforms? Is the package well maintained
  • Check that any changes are related to the issue at hand.

@PartyDonut PartyDonut left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me thanks for helping out.

Left some small requests.

Currently the offset is remembered even when stopping/starting playback. This should probably be reset after the video is done playing?

VideoHotKeys.takeScreenshot => context.localized.takeScreenshot,
VideoHotKeys.takeScreenshotClean => context.localized.takeScreenshotClean,
VideoHotKeys.toggleSubtitles => context.localized.toggleSubtitles,
VideoHotKeys.subtitleOffsetBackward => 'Subtitle backward',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These should be localized in the app_en.arb base translations file.

String _subtitleDelayLabel(Duration offset) {
final absMilliseconds = offset.inMilliseconds.abs();
final sign = offset.inMilliseconds >= 0 ? '+' : '-';
return 'Subtitle $sign${absMilliseconds}ms';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should also be translated

final mediaSegments = ref.read(playBackModel.select((value) => value?.mediaSegments));
final position = ref.read(mediaPlaybackProvider).position;
final playing = ref.read(mediaPlaybackProvider.select((value) => value.playing));
final subtitlesEnabled = ref.read(playBackModel)?.mediaStreams?.defaultSubStreamIndex != null &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could just show the subtitle offset always, not sure we have to hide it if nothing is selected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants