khenriks/mp3fs

"V" support for lame

Closed this issue · 14 comments

I only see fixed bitrates for the mp3 conversion. Would it be possible to add the variable bitrates? (Such as the common recommendations seen here:
http://wiki.hydrogenaudio.org/index.php?title=LAME#Recommended_encoder_settings )

This can be done, although the nice thing about CBR encoding is that the file size can be predicted before actually doing the encode. If I implement VBR encoding, this would mean the file size from ls could only be reported as "0".

What would the consequences be by reporting estimated file sizes (or zero) rather than the exact sizes?

Reporting an incorrect estimate (zero or otherwise) could create a problem with programs which rely on the size, which actually was a problem I encountered in the past with inaccurate estimates. For example, rsync will by default only pay attention to a file's reported size. It will stop before the end of the file if it reaches the reported length; or if the actual length is less than the reported length, it may report an error when it can't read as many bytes as it wants. I haven't looked to see if this is possible to change with a command-line option to rsync, but it's an unfortunate effect of incorrect size reporting.

I added some code that adds variable bit rates. It is not ideal, in that it converts the entire file to an MP3 just to get the file size. I added a caching layer that caches the MP3 and the file sizes so that the conversions only have to happen once.

Are you interested in some or all of these changes, i.e. just the caching layer, the whole thing?

The variable bit rate files would have to just be reported with size 0. Caching is beyond the scope of mp3fs, and it would be bad if ls -l on a directory took over a minute, even if that's only the first time.

I would be interested in the changes though, just for enabling VBR files.

I am curious why you think that caching is beyond the scope of mp3fs. Caching is just a performance enhancement, and is used in lots of file systems.

Well, perhaps it's better to say it's beyond the scope right now. It has a simple architecture at the moment, which is very nice and makes it easy to deal with. When you add a cache, there's the question of how to handle all of that. Where to plug it into the existing design. And complicated issues like when to expire the cache.

There is still the problem of having to transcode an entire directory full of files just as a result of ls -l. That command should return very quickly; delays there are not good.

That makes sense. Let me figure out how to break out just the VBR part out of what I have done and give that to you first. Then, I can give the caching parts later, and you can do with it what you want.

Okay, that sounds good to me.

It looks like reporting a zero size won't work. It appears that something (maybe in fuse?) is seeing that the file size is zero, so anything returned by mp3fs_read() is truncated by that size. If I make the reported size be 128, then a read of the file (I am using 'od') returns 128 bytes.

As an alternative to reporting zero, we could have it report MAX_INT, which is even more misleading than reporting zero, but it appears to make everything else work.

Never mind. Some MP3 readers (at least Juk does this) will try to read the tag at the end of the file, and uses the size to determine the end of the file. So, it ends up reading off the real end of the file, so the tag data it displays is wrong.

So, trying to fake the file size causes problems no matter what.

Yeah, trying to do any kind of format where the output size is unknown just causes all sorts of problems like this, because you can't know the true size without doing the actual encoding, and lying about the size causes problems.

There is something you can do, which is not ideal and kind of ruins the space savings of VBR, but you can lie about a size that you know for certain is too large, and then at the end you just pad the output file with zeroes, and produce a file that is the expected size, even if the audio data has a different size.

This was done in 0f1cff9.