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

Implicit conversion from float to int loses precision (Parser/SilenceParser.php line 61) #247

Open
brandonscript opened this issue Oct 22, 2023 · 1 comment

Comments

@brandonscript
Copy link

I'm running this via the auto-m4b Docker container, and in several cases I'm getting:

an error occured, that has not been caught:
Array
(
    [type] => 8192
    [message] => Implicit conversion from float 1000313.76 to int loses precision 
    #                                                                         ^ (or some variation on this value)
    [file] => phar:///usr/local/bin/m4b-tool/src/library/M4bTool/Parser/SilenceParser.php
    [line] => 61
)

I figured this was related to looking for chapter silence with --max-chapter-length=15,30 but oddly I've tried without this option entirely and it still seems to crop up.

Best I can tell it's related not to line 61 (there's nothing there) but to one of the TimeUnit or millisecond() calls there – looking at https://github.com/sandreas/php-time/blob/master/src/Sandreas/Time/TimeUnit.php, it looks like it's supposed to handle both float and int values, but since this is compiled running in Docker, I don't really know how to test or troubleshoot. If anyone can tell at a glance what might be going on, or suggestions on debugging, please let me know.

Past that, I'm not sure if this is philosophically a bug in TimeUnit, or whether the values should just be rounded to ints in SilenceParser source here, like:

// Round down and convert to int to ensure we don't cut any sound off
$end = new TimeUnit((int)floor($matches[1]), TimeUnit::SECOND);
// Again, round down the duration to avoid loss and convert to int
$silenceDuration = new TimeUnit((int)floor($matches[2]), TimeUnit::SECOND);
// Start should be rounded up
$start = new TimeUnit((int)ceil($end->milliseconds() - $silenceDuration->milliseconds()), TimeUnit::MILLISECOND);

$this->silences[$start->milliseconds()] = new Silence($start, $silenceDuration);
@sandreas
Copy link
Owner

@brandonscript Same here like in #248: PHP 8.1 handles some things more strict. This is an external reference where I can't do much without having the possibility of a truncated value or updating the external reference.

Try to do that someday, but for now this is tricky since I'm using this library in many of my smaller side projects and the impact would be too big.

Another hint: --max-chapter-length uses milliseconds, so --max-chapter-length=15,30 would result in chapters being 15ms - 30ms... maybe this results in the error?!

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

No branches or pull requests

2 participants