sandreas / m4b-tool

m4b-tool is a command line utility to merge, split and chapterize audiobook files such as mp3, ogg, flac, m4a or m4b
MIT License
1.12k stars 78 forks source link

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

Open brandonscript opened 8 months ago

brandonscript commented 8 months ago

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 commented 8 months ago

@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?!