Skip to content

Conversation

@marcybelardo
Copy link

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.
  • I have mentioned this change in the changelog.

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

For issue #1483, refactors formatting of col1 and row1 variables to avoid issues with LC_NUMERIC auto-formatting float values.

Decided to use %03u to keep precision at 3 digits, used %03u over %.3u for better code readability in the format string.

Refactors formatting of `col1` and `row1` variables to avoid issues with LC_NUMERIC autoformatting float values
TTML specifications make no recommendation regarding decimal places for tts:origin values. Tests were done on video and no discernable difference was observed.
Will return integer and fractional parts of the floats, but no longer requires 3 decimal places at all times, can show less, but 1 at minimum.
@cfsmp3
Copy link
Contributor

cfsmp3 commented Mar 22, 2023

I've been experimenting with this - I think using the proper locale for numeric is the best way to go, otherwise we'll have to change lots of lines of code just to get what the C library gives us for free. I pushed a one-liner comment to use the POSIX locale.

Here's a trivial test:

#include <stdio.h>
#include <locale.h>

int main() {
    float col1 = 3000000000000;
    setlocale (LC_ALL, "en_DK.utf8");
    printf ("%1.3f\n", col1);
    setlocale (LC_NUMERIC, "POSIX");
    printf ("%1.3f", col1);
}

Which outputs:

3000000053248,000
3000000053248.000

Hopefully this will fix this issue everywhere.

@cfsmp3 cfsmp3 closed this Mar 22, 2023
@marcybelardo
Copy link
Author

I've been experimenting with this - I think using the proper locale for numeric is the best way to go, otherwise we'll have to change lots of lines of code just to get what the C library gives us for free. I pushed a one-liner comment to use the POSIX locale.

Here's a trivial test:


#include <stdio.h>

#include <locale.h>



int main() {

    float col1 = 3000000000000;

    setlocale (LC_ALL, "en_DK.utf8");

    printf ("%1.3f\n", col1);

    setlocale (LC_NUMERIC, "POSIX");

    printf ("%1.3f", col1);

}

Which outputs:


3000000053248,000

3000000053248.000

Hopefully this will fix this issue everywhere.

Oh I see! Right, I was thinking that fixing every sprintf eventually would be tedious. Well, thank you for your help with all this!

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