dmulholl / mp3cat

A command line utility for joining MP3 files.
https://www.dmulholl.com/dev/mp3cat.html
The Unlicense
185 stars 22 forks source link

I renamed an .RTF File to MP3, just to see if MP3Cat would Complain or even notice. #10

Closed RobertBBrownell closed 6 years ago

RobertBBrownell commented 6 years ago

I renamed an .RTF File to MP3, in with my other valid MP3s, just to see if MP3Cat would Complain or even notice. No Complaint, Didn't even notice, and the Errorlevel returned was still 0.

We need to get some basic checking.

dmulholl commented 6 years ago

Hi Robert. I can't return an error code in this situation or in the case of differing input bit-rates as these are actually perfectly legitimate input.

The program will happily concatenate MP3 files with different bit rates or any mixture of constant-bit-rate and variable-bit-rate files.

It works by scanning the input files for individual MP3 frames and copying them to the output file. It then adds a VBR header if it detects multiple bit-rates in the input.

This means it will skip over any ID tags or non-MP3 data in the input files, e.g. embedded cover images, non-standard metadata, etc. Your RTF file will simply have been scanned for MP3 frames and then ignored - the program doesn't pay any attention to file extensions.

I built a simple MP3-scanning library as a backend for this tool - you could use that to build a dedicated tool for detecting differing bit-rates. Here's the link:

https://github.com/dmulholland/mp3lib

ohadschn commented 6 years ago

Well you have tools like Robocopy with non-zero success exit codes. You could do something like: 0 - success 1 - success, different bitrates 2 - success, some files were ignored 4 - error (note the powers of two to allow multiple exit codes, e.g. 3 would mean success, different bitrates + some files were ignored)

dmulholl commented 6 years ago

The program already reports when multiple bitrates are detected as part of its standard output.

I'm not dogmatic about sticking to conventions but the problem with breaking them is that you need to document it prominently to avoid surprising people. And realistically, how many users are ever going to check for an exit code meaning 'success, but some of the input files contained no mp3 frames'? Compare that to everybody who ever uses the tool having to read the expanded documentation just in case it might be something they need to know.

I'm more in favour of keeping individual tools as simple as possible and building specialized tools for specialized jobs.

I do like the bitmasking idea for exit codes though, it's clever.

ohadschn commented 6 years ago

The way I see it, standard output is nice for human-readable information, but reliable scripting requires exit codes. Presumably, anyone using exit code should read the docs for the tool they are using (like you would read the man for grep when you use it). It's basically part of the contract, much like the accepted arguments. Of course, you could argue that people don't do what they're supposed to :)

Can't take credit for the bitmasking exit code BTW: https://ss64.com/nt/robocopy-exit.html