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

Fix reporting time in foobar2000 over 24 hours #14570

Merged
merged 9 commits into from
Feb 13, 2023

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Jan 19, 2023

Link to issue number:

Fixes #14127
Supersedes #14451

Summary of the issue:

When using the foobar2000 audio player, the ctrl+shift+r command should report the remaining time in the audio file in the SS, MM:SS, HH:MM:SS, or D HH:MM:SS format depending on the remaining time length.
However, using the command on audio files over a day long produces an error as the code attempts to read the time string in the HH:MM:SS format instead of the D HH:MM:SS format.

Description of user facing changes

Time calculation is now processed correctly for all audio length up to 31 days. After this point, it is uncertain how foobar reports time.

In addition to fixing the time calculations, the spoken UI message was also amended to say "[time] total", "[time] elapsed", or "[time] remaining", rather than just "[time]" so as to be more understandable for the listener.

Description of development approach

Created a collection of translatable time string output formats.

Created a generic way to parse timedeltas to localised formatted strings.

Created internal functions for foobar appModule to parse foobar time strings to localised formatted strings.

Testing strategy:

  • Manual testing (need confirmation):
  1. Open foobar2000
  2. Open audio file over 24 hours long
  3. Skip to multiple time points throughout audio file to test out following steps 4-6:
  4. Type ctrl + shift + e and confirm that correct elapsed time is reported
  5. Type ctrl + shift + r and confirm that correct remaining time is reported
  6. Type ctrl + shift + t and confirm that correct total time is reported
  7. Test out commands with audio files under 24 hours long as well
  • Unit testing
    Unit testing has been added

Known issues with pull request:

It is uncertain how reporting works for files over a month long.

Change log entries:

Bug fixes
- Remaining, elapsed and total time is now reported correctly for audio files over a day long in foobar2000. (#14127)

API breaking changes
- The following symbols have been removed from ``appModules.foobar2000`` with no direct replacement.
  - ``statusBarTimes``
  - ``parseIntervalToTimestamp``
  - ``getOutputFormat``
  - ``getParsingFormat``
  -

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
    • After merging: Announce API breaking change on add-on API mailing list
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd requested a review from a team as a code owner January 19, 2023 05:17
@seanbudd seanbudd added this to the 2023.1 milestone Jan 19, 2023
@seanbudd seanbudd force-pushed the fixup-foobar2000-reporting branch from 4c68bc4 to fac1f98 Compare January 19, 2023 05:20
@CyrilleB79
Copy link
Collaborator

In change log:
2000 should be written instead of 200 (a zero is missing)

@seanbudd seanbudd merged commit f2ce95b into master Feb 13, 2023
@seanbudd seanbudd deleted the fixup-foobar2000-reporting branch February 13, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants