search

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.14k stars 77 forks source link

Multiple dependency errors after brew upgrade #171

Open xtal-ball opened 2 years ago

xtal-ball commented 2 years ago

I updated my Homebrew installation recently on MacOS 11.5.2 and went from PHP 8.0.3 to 8.1.2, which started giving the warnings and errors reported in issue #169. I updated m4b-tool to version latest-137-g14781fb. I can confirm that disabling deprecation warnings by editing /usr/local/etc/php/8.1/php.ini helps, though various uncaught PHP errors still occur including AbstractCommand.php:420 trim(): Passing null to parameter #1 ($string) of type string is deprecated and TimeUnit.php:241 Return type of Sandreas\Time\TimeUnit::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

Also concerning is the error I get from m4b-tool when I start a real command (merge): ffmpeg version 4.0.0 or higher is required - installed version 2.0.5 is likely to cause errors or unexpected behaviour.... The Homebrew upgrade also moved ffmpeg from 4.3.2 to 5.0:

% ffmpeg -version
ffmpeg version 5.0 Copyright (c) 2000-2022 the FFmpeg developers
built with Apple clang version 12.0.5 (clang-1205.0.22.11)
configuration: --prefix=/usr/local/Cellar/ffmpeg/5.0-with-options --enable-shared --cc=clang --host-cflags= --host-ldflags= --enable-gpl --enable-libaom --enable-libdav1d --enable-libmp3lame --enable-libopus --enable-libsnappy --enable-libtheora --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libx264 --enable-libx265 --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-demuxer=dash --enable-opencl --enable-videotoolbox --disable-htmlpages --enable-libfdk-aac --enable-libwebp --enable-libxvid --enable-nonfree
libavutil      57. 17.100 / 57. 17.100
libavcodec     59. 18.100 / 59. 18.100
libavformat    59. 16.100 / 59. 16.100
libavdevice    59.  4.100 / 59.  4.100
libavfilter     8. 24.100 /  8. 24.100
libswscale      6.  4.100 /  6.  4.100
libswresample   4.  3.100 /  4.  3.100
libpostproc    56.  3.100 / 56.  3.100

I did a little digging and I see that symptom comes from your version parsing expecting a dotted triplet of single digits, so "5.0" gets ignored and "clang version 12.0.5" gets truncated to 2.0.5 to produce the warning message. That may be simple enough to fix (add "ffmpeg version" to your matching pattern and make the third digit optional), but I worry that there may be other deeper incompatibilities lurking with a new major version.

I haven't done any extensive testing, but I did rebuild one of my books from the same input files and the final output went from 17MB to 27MB (for two chapters as m4a inputs, 12MB and 8MB). So something is funky and it's probably worth digging deeper. I'll try to revert to the older ffmpeg for now.

sandreas commented 2 years ago

Thanks for reporting and giving so much info.

haven't done any extensive testing, but I did rebuild one of my books from the same input files and the final output went from 17MB to 27MB (for two chapters as m4a inputs, 12MB and 8MB).

Well that is a major problem. I plan to dig into this and get back to m4b-tool development, unfortunately JetBrains has canceled my Open Source license for PHP Storm (no offense, I did not do anything in the last months, so it is not their fault) and I'm trying out new IDEs atm - which takes more time that I wanted to spend. But I think m4b-tool will get a little love in the next weeks.

sandreas commented 2 years ago

Ok, I did some further investigation.

The deprecation warnings were shown by a custom shutdown handler - this should be fixed in the latest code, but there is no release for this yet.

The version parsing is ok in the latest code - the regex has changed to 

preg_match("/^.*([0-9]\.[0-9]\.[0-9]|[0-9]\.[0-9]).*$/sU", $output, $matches);

I would recommend to use the latest pre-release, if this not already is the case. See here for more information: https://github.com/sandreas/m4b-tool/releases/tag/latest

This should fix all of your problems and bring some new features in.

xtal-ball commented 2 years ago

Thanks for taking a look so quickly. I thought I had previously grabbed the latest prerelease, but I may be misunderstanding something. https://github.com/sandreas/m4b-tool/releases/tag/latest shows the latest prerelease link as https://github.com/sandreas/m4b-tool/files/6598116/m4b-tool.tar.gz , using the same link in the example docker build line, Mac/Linux download & extract text, and in the historic list by dates as being from 2021-06-04. That's the one I had previously updated to, which reports itself as

% ~/bin/m4b-tool.phar --version
m4b-tool latest-137-g14781fb

Also at the releases/tag/latest page near the top I see sandreas released this Nov 23, 2019 · 82 commits to master since this release latest cb0b121, and clicking the 82 link I do indeed see a bunch of new commits including the most recent 51bf1f0 wherein you squash the deprecation warnings. But I don't follow how to find the link to a more recent tgz of a .phar you built from the latest commit (which is clearly not the one with 6598116 in Github's file URL).

I don't know anything about PHP but I found your build instructions and with the addition of brew install composer; composer update I was able to make my own .phar from the tip of master. It reports its version accurately to that commit, but without deprecated errors rejected in my php.ini they still print a lot:

% ./dist/m4b-tool.phar --version
PHP Deprecated:  Return type of Sandreas\Time\TimeUnit::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///Users/redacted/m4b-tool/dist/m4b-tool.phar/vendor/sandreas/php-time/src/Sandreas/Time/TimeUnit.php on line 241

Deprecated: Return type of Sandreas\Time\TimeUnit::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///Users/redacted/m4b-tool/dist/m4b-tool.phar/vendor/sandreas/php-time/src/Sandreas/Time/TimeUnit.php on line 241
m4b-tool latest-82-g51bf1f0

Note that they are not printing at exit in the shutdown function you tweaked in 51bf1f0, but rather as the program executes. My merge command spammed 702 deprecated lines while assembling a book out of two input chapters -- I believe each warning is printed twice, once with PHP Deprecated: and once with Deprecated: prefixes, so that was only 351, but still. 23 of them were unique, and I'll share on request if you have trouble reproducing but want to squash them individually rather than just masking the logging.

When I disable deprecated errors again in php.ini, all of that goes away and the execution looks clean.

I do confirm that with the local build I made there is no longer any complaint about the version string from ffmpeg 5.0, and in double checking things I'm pretty sure I followed your readme a while back to add fdk-aac to my brew ffmpeg (and it's still there with my 5.0). The increase in output file sizes remains, and I've found it difficult to coerce an older version of ffmpeg back into existence, but for the moment I'm going to assume the file size is entirely unrelated to m4b-tool and not your problem.

Thank you very much for this tool - it's been a great help to me. What I don't see in your readme is how I can buy you a beer...

mtwebb commented 2 years ago

Happy to find this thread in progress. I'm in the same camp as @xtal-ball. If latest tarball can be updated @sandreas , I will happily test and report back. m4b-tool is a very nice set of tools. Thank you for creating and sharing.

sandreas commented 2 years ago

I do confirm that with the local build I made there is no longer any complaint about the version string from ffmpeg 5.0, and in double checking things I'm pretty sure I followed your readme a while back to add fdk-aac to my brew ffmpeg (and it's still there with my 5.0). The increase in output file sizes remains, and I've found it difficult to coerce an older version of ffmpeg back into existence, but for the moment I'm going to assume the file size is entirely unrelated to m4b-tool and not your problem.

The increasing size problem might be a change in the default value for --audio-bitrate. Maybe you could try

 --audio-channels=1 --audio-bitrate=64k --audio-samplerate=22050

Thank you very much for this tool - it's been a great help to me. What I don't see in your readme is how I can buy you a beer...

If latest tarball can be updated @sandreas , I will happily test and report back. m4b-tool is a very nice set of tools. Thank you for creating and sharing.

Thank you, I really appreciate it, that's what keeps me going. As I already stated out in issue #87, I'm in the lucky situation that I personally do not need to earn extra money and I'm doing this project for fun - as long as my spare time allows me to do so. But as a little gift for your kind words, let me give you a short update about my personal situation:

The good news

Let's see what it takes to get there, in the meantime, you unfortunately have to wait for the next pre-release... I'm on it :-)

JohnKacz commented 2 years ago

Wow! Thanks for that insight @sandreas and good luck with tone and C#.

mtwebb commented 2 years ago

Wow, where's Github's like and subscribe buttons when I need them? What a fantastic update. Glad to hear about the JetBrains license and the new tone project. Good luck with the multiple endeavors. I'll keep my eyes peeled for the update later this year.

xtal-ball commented 2 years ago

Thanks for the personal update @sandreas! Good news on JetBrains, and tone sounds fun! My primary usage of m4b-tool is making audiobook files out of recordings of reading long books to my kids (for their repeated consumption), so my thanks for the tool are quite personal.

Back on the the ticket topic, sorry it's taken me a little while to find the time to experiment enough to have anything to report. I think there are two main issues we've been discussing -- one is deprecated warnings, the other is file growth. On the deprecated warnings front, this is the only PHP CLI utility I run, so I don't mind keeping deprecated warnings disabled in php.ini; it's certainly a sufficient workaround for me (and it's still needed with 500a5b0).

On the file size front, I got back to a place where I could quickly switch back and forth between FFMpeg 4.3.2 and 5.0, and I reran a merge many different times with different arguments and different FFMpeg versions. My past typical workflow was using --audio-quality 80 for VBR encoding, which seems to have been the proximate cause of my troubles. I tried variable bitrate at quality 80 and 100, plus your suggestion of --audio-bitrate 64k, and I mixed in 22050, 44100, and 48000 sample rates. I did a bunch of comparison with all the numbers, but I doubt anyone needs all the details here. The main thing that changed between FFMpeg 4.3.2 and 5.0 is how high the average variable bitrate is for a given --audio-quality level. If I just abandon that and use a fixed bitrate like you suggest, then the differences between versions are minimal.

I think that means there isn't much to do here, but I'll leave the ticket open so that others wanting to mask the deprecated warnings will find it more easily.

One thing that would definitely help others is if you updated the latest prerelease binary to match the current 500a5b0. I can still only run that code because I cloned the repo and built it myself. It's unclear to me if the JetBrains thing was blocking that.

sandreas commented 2 years ago

so my thanks for the tool are quite personal.

Thank you, I am also recording personal book readings for my daugther :-) Maybe you also wanna take a look at my (very small and early) side project: https://pilabor.com/projects/labelmaker/ - It is basically a label generator tool for an RFID based music box, that my daugther uses to play audio books herself just putting a wooden card on a marked rectangle on the box. The article also contains a photo of the box, an explanation of the principle and further links.

The main thing that changed between FFMpeg 4.3.2 and 5.0 is how high the average variable bitrate is for a given --audio-quality level. I think that means there isn't much to do here, but I'll leave the ticket open so that others wanting to mask the deprecated warnings will find it more easily.

That is good to know, but I agree, that there is not much I can do here. Thank you for putting effort into this. Unfortunately, there is hardly a way around ffmpeg, it is the most versatile open source cross platform tool to encode audio, that I could find. Maybe someday I'll add / replace it with sox, which seems to be much more reliable for audio stuff. We'll see.

One thing that would definitely help others is if you updated the latest prerelease binary to match the current https://github.com/sandreas/m4b-tool/commit/500a5b0b7140742851cafa66a1f6b9b24668e68b. I can still only run that code because I cloned the repo and built it myself. It's unclear to me if the JetBrains thing was blocking that.

Yes, definitely. I am currently experimenting with github actions to perform a fully automated release cycle based on git tags, but for a PHP project this old and having so many dependencies, it is not that easy to do in a reliable form. That's why I started to experiment with tone. For .NET projects this seems to be a lot easier to do. Another reason for starting tone was getting rid of mp4v2, which is a reliable and somewhat awesome toolset, but the current development and future of the project is questionable (currently I have to maintain a personal fork to support some of the m4b-tool features I personally need). A pure .NET replacement for mp4v2 will probably be https://github.com/Zeugma440/atldotnet, which so far looks more than awesome to me. I would love to see more github stars and supporters here.

xtal-ball commented 2 years ago

there is hardly a way around ffmpeg

Indeed not, it has a wonderful feature set but is rather difficult to use. And on that note, I couldn't leave well enough alone, and I read more about VBR versus CBR in the FFmpeg AAC docs. I also looked at the debug output from m4b-tool and realized that it has been using -acodec aac -q:a 1.6 when I had asked for --audio-quality 80, rather than -acodec libfdk_aac -vbr 4. If I work around the bug preventing Fraunhofer detection (by forcing --audio-codec libfdk_aac) and go back to 80% VBR quality, the FFmpeg 5.0 suite with libfdk_aac generally chooses good bitrates in the 80k range and gives me nice output file sizes.

It's possible that what actually occurred was that my previous setup with an old m4b-tool version was successfully using FFmpeg 4.x's libfdk_aac at 80% quality but then in the upgrade and diagnostics shuffle I ended up with the newest m4b-tool using FFmpeg 5.0's native AAC encoder with its experimental VBR that is uncomfortably large. I don't have enough old logs or binaries to prove it, I just happened to stumble on this possibility.

Good luck with the automated github releasing, I hope that works out well!

sandreas commented 2 years ago

If I work around the bug preventing Fraunhofer detection (by forcing --audio-codec libfdk_aac) and go back to 80% VBR quality, the FFmpeg 5.0 suite with libfdk_aac generally chooses good bitrates in the 80k range and gives me nice output file sizes.

If you care about quality / low sizes and are willing to experiment, you should give fdkaac a shot. See:

fdkaac produces WAY better results than ffmpeg and is only slightly slower due to piping the output through ffmpeg before encoding. It also has less problems with bogus source files.

Use --audio-channels=1 --audio-bitrate=64k --audio-samplerate=22050 --audio-codec=libfdk_aac --audio-profile=aac_he for initial testing.