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

wvtag fails to open some valid files without the .wv extensions #79

Open
desbma opened this issue Jan 12, 2020 · 5 comments
Open

wvtag fails to open some valid files without the .wv extensions #79

desbma opened this issue Jan 12, 2020 · 5 comments

Comments

@desbma
Copy link

desbma commented Jan 12, 2020

Copy a valid WavPack file to /tmp/f, /tmp/1, /tmp/1.wv.

/tmp/f is readable as expected:

$ wvtag -q -l /tmp/f

APEv2 tag items:   1 (93 bytes used)
encoder:           Lavf58.29.100

/tmp/1 is not:

$ wvtag -q -l /tmp/1
can't open file

strace shows that wvtag tries to open /tmp/1.wv instead...

And if I add a .wv file extension, it works:

$ wvtag -q -l /tmp/1.wv

APEv2 tag items:   1 (93 bytes used)
encoder:           Lavf58.29.100

I'm on ArchLinux with latest WavPack:

wvtag --version
wvtag 5.2.0
libwavpack 5.2.0
@dbry
Copy link
Owner

dbry commented Jan 12, 2020

I tried on a couple Linux distros and was not able to reproduce the exact behavior you're seeing. Are you sure you don't have some "f"s and "1"s mixed up?

That said, however, there is an issue with all the WavPack command-line programs in that they cannot handle filenames without extensions because if a filename is entered without an extension they add a default (like .wv in your case). See the wvtag usage:

WVTAG [-options] file[.wv] [...]

This behavior is not very Linuxy but it came from Windows. Is this a showstopper in some way? The only solution I can think of that wouldn't break a lot of stuff would be add an option to force filenames to be interpreted literally (--literally or --literal-filenames or --no-default-extensions).

@desbma
Copy link
Author

desbma commented Jan 12, 2020

Are you sure you don't have some "f"s and "1"s mixed up?

Indeed I must have mixed up my test, all files without extensions cause the error.

That said, however, there is an issue with all the WavPack command-line programs in that they cannot handle filenames without extensions because if a filename is entered without an extension they add a default (like .wv in your case). See the wvtag usage:

WVTAG [-options] file[.wv] [...]

That line tells me the .wv extension is optional, so files without it should be accepted. Also on my system man wvtag shows: wvtag [-options] INFILE....

This behavior is not very Linuxy but it came from Windows. Is this a showstopper in some way?

I love WavPack, and this is free software so I can't complain. 😉
But this is definitely weird behavior unexpected for such a program. I don't know any command line audio tool that does that.

The only solution I can think of that wouldn't break a lot of stuff would be add an option to force filenames to be interpreted literally (--literally or --literal-filenames or --no-default-extensions).

I'm not sure the added complexity and flags would not be worse that the current behavior.

In other words, I think the sane default behavior would be to not add an extension if the input file name does not have any, but if you can't change it for legacy reasons, then so be it, you can close this issue.

@dbry
Copy link
Owner

dbry commented Jan 14, 2020

That said, however, there is an issue with all the WavPack command-line programs in that they cannot handle filenames without extensions because if a filename is entered without an extension they add a default (like .wv in your case). See the wvtag usage:
WVTAG [-options] file[.wv] [...]

That line tells me the .wv extension is optional, so files without it should be accepted.

What that line means is that specifying the extension is optional. I can't understand why it would be good to have WavPack files with the extensions removed. Is that a thing?

I just looked and I could add simply -l (except for wvtag, which uses that) to accept filenames literally, and it really doesn't add too much complexity because it would only apply to someone who ran into this issue. I will consider that for the next release.

And yeah, I can't just change the behavior to not add the extension because that would break the world. And checking with and without adding the extension sounds like a dangerous hack; what if both exist?

BTW, this is an alternative if you really get stuck:

$ wvtag -l - < /tmp/1

@desbma
Copy link
Author

desbma commented Jan 14, 2020

What that line means is that specifying the extension is optional.

So it allows passing a file name that is not the input file name.

I can't understand why it would be good to have WavPack files with the extensions removed. Is that a thing?

Nothing is removed. I pass a filename x and somehow wvtag decides that it is smarter than me, that it should definitely rather be x.wv, and it adds an extension implicitly, which causes it to fail for no good reason.

It's generally a good thing for programs to assume the user knows what he/she is doing. Especially if this is a command line program, I mean this is not a phone app with autocomplete where you should favor typing ergonomics and such.

If I followed this flawed logic, I suggest you also hardcode the output directory as ~/Music because after all, people are encoding music, right? What could possibly go wrong, it's definitely a nice feature to have!

Also you seem to be missing that command line programs are most likely called from other programs (scripts) rather than by users.

If you want the whole context for my use, here it is:

I have a program for my own use that takes an input audio track (that can be in a lossy or lossless format), applies a bunch of complex audio transformations, and then plays the result. Because the transformation pipeline is slow, the result is saved in a cache, as Vorbis if the input track is a lossless file (to save space), or as WavPack if input is lossy (to avoid a second lossy conversion and degrade quality). The file name in the cache is a MD5 hash of all the transformation parameters, so its for example something like this 79/aeb79bffc923fd2e54416207090941.

Then next time I run the program with the same parameters, the cache file is just played. I don't have an extension because the file name must be the same if the format is Vorbis or WavPack. Also an extension would be completely useless anyway because the player (mpv/ffmpeg) does not rely on it at all.

I just looked and I could add simply -l (except for wvtag, which uses that) to accept filenames literally, and it really doesn't add too much complexity because it would only apply to someone who ran into this issue. I will consider that for the next release.

Honestly do what you feel is right, I can work around this issue easily.

To me it seems also wrong to have an opt in switch to restore a sane behavior, and I don't think I have ever met a command line program that implicitly changes the input path.

So if there is no solution you can safely close this issue, I won't be mad, and I value you work.

@dbry
Copy link
Owner

dbry commented Jan 21, 2020

I'll leave the issue open for now because not being able to specify an input file that does exist is a bug. Surprisingly nobody has complained until now, but ironically I ran into the issue integrating with oss-fuzz because its "crasher" files have no extensions, so I have to rename them to feed them into wvunpack. That said, I'm unwilling to change the current behavior because of what might break in the wild and it is definitely an edge case. Thanks for pointing it out.

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