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

Convert MP3 default isn't gapless #1741

Closed
smlx opened this issue Nov 30, 2015 · 5 comments
Closed

Convert MP3 default isn't gapless #1741

smlx opened this issue Nov 30, 2015 · 5 comments
Labels

Comments

@smlx
Copy link
Contributor

smlx commented Nov 30, 2015

When importing losless files, the convert plugin's default mp3 command-line discards length information:

ffmpeg -i $source -y -vn -aq 2 $dest

Instead, I believe that the following command-line should be default:

ffmpeg -i $source -f wav - | lame -V 2 --noreplaygain - $dest

This ensures that delay/padding information is preserved in the LAME MP3 Info Tag, enabling gapless playback on many devices / applications. The downside, of course, is that the lame executable is required in addition to ffmpeg.

Would you accept a pull-request to change the default command-line, or at least document this shortcoming of the default and the alternative gapless solution?

@sampsyo sampsyo added the docs label Nov 30, 2015
@sampsyo
Copy link
Member

sampsyo commented Nov 30, 2015

Thanks for pointing that out; good point. Let's document it. It's nice that, for now, it's easy to describe the dependencies for the default convert configurations (you just need ffmpeg). I'm also not sure we currently support shell pipelines in the configuration, so you might need to wrap this in a shell script to get it to work.

@smlx
Copy link
Contributor Author

smlx commented Nov 30, 2015

I'm actually using that exact configuration line with the pipe, so it does work. 😄
Okay, I'll look at documenting this.

@sampsyo
Copy link
Member

sampsyo commented Nov 30, 2015

Well, that's great, at least!

@smlx
Copy link
Contributor Author

smlx commented Nov 30, 2015

There's a small wrinkle with this: I'm using beet 1.3.8 from Debian, which allows the pipe in the command.

As part of #1157 (in 8579412), you removed this capability. As there is no safe way of handling a malicious $source or $dest, I guess we'll need a standalone shell script to handle this case...

@sampsyo
Copy link
Member

sampsyo commented Dec 1, 2015

Got it; good point. Yes, we'll need to recommend a shell script wrapper (or add some crazy special handling for shell syntax).

sampsyo added a commit that referenced this issue Jun 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants