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

[Video][Actions] new action: PreviousSubtitle #24943

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

foresto
Copy link

@foresto foresto commented Apr 5, 2024

Background

When advancing through the available subtitle tracks, it's easy to accidentally overshoot the desired one, and end up having to either navigate menus to go back a step (obscuring the video) or keep advancing through the entire list to wrap around to the beginning and finally reach the desired one again (an annoyingly circuitous approach).

Also, there are times when switching back and forth between two adjacent subtitle tracks makes sense. For example, when watching a bilingual film with a few bits of muddled dialogue, the viewer might want the normal subtitles for most of the film but switch to SDH subtitles for the muddled parts, and then switch back again. Again, this is cumbersome when the only options are to navigate menus or advance forward through the subtitle track list.

In both these cases, the problem is especially bad when subtitles for dozens of languages are present.

What does this change?

This patch adds the PreviousSubtitle action, for symmetry with NextSubtitle, and to handle situations like those described above with a single button press.

How has this been tested?

Tested on (and originally written for) Leia, on a Raspberry Pi 2 running OpenELEC. (Because that's what I have available for testing.) I mapped a button to the PreviousSubtitle action and verified that it behaves like NextSubtitle, but in the other direction.

Porting to the master branch was straightforward, since the affected code has had no structural changes since Leia.

What is the effect on users?

No effect on users unless they (or their distribution) map a button to this new action. In that case, they get a quality-of-life improvement.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • Student submission (PR was done for educational purposes and will be treated as such)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

When advancing through the available subtitle tracks, it's easy to
accidentally overshoot the desired one, and end up having to either
navigate menus to go back a step (obscuring the video) or keep advancing
through the entire list to wrap around to the beginning and finally
reach the desired one again (an annoyingly circuitous approach).

Also, there are times when switching back and forth between two adjacent
subtitle tracks makes sense. For example, when watching a bilingual film
with a few bits of muddled dialogue, the viewer might want the normal
subtitles for most of the film but switch to SDH subtitles for the
muddled parts, and then switch back again. Again, this is cumbersome
when the only options are to navigate menus or advance forward through
the subtitle track list.

In both these cases, the problem is especially bad when subtitles for
dozens of languages are present.

This patch adds the PreviousSubtitle action, for symmetry with
NextSubtitle, and to handle situations like these with a single button
press.
@foresto foresto requested a review from garbear as a code owner April 5, 2024 20:30
@foresto foresto changed the title [Actions] add PreviousSubtitle [Actions] new action: PreviousSubtitle Apr 5, 2024
@foresto foresto changed the title [Actions] new action: PreviousSubtitle [Video][Actions] new action: PreviousSubtitle Apr 5, 2024
@foresto
Copy link
Author

foresto commented Apr 28, 2024

I don't think I have permission to satisfy the validator's MILESTONE and LABEL checks. Is there something I can do about those failing?

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.

None yet

1 participant