Corey-M/NAudio.Lame

LameMP3FileWriter encodes with random bitrate value when using FileStream.Create to generate a File (Stream)

Closed this issue · 6 comments

First of all, thanks a lot for this lib it's really useful,

So, I upgraded from 1.0.9 to 1.1.6, after upgrading NAudio to 1.10.0
Looks like it does not use the bitrate set when using Stream, when using string (path to file) it works:

  • Using: Bit rate used ABR_160
  • Issue: Instead of getting mp3 around 155-165 like before, it now goes between 20 to 190

code when I was using 1.0.9

// fileName: string pathname to file ("..\fileName.mp3")
// supportedWaveFormat : Naudio.WaveFormat (48kHz sample rate, 2 channels)
// _userSettings.Bitrate : LAMEPreset (one of those 4 : ABR_128, ABR_160, ABR_256, ABR_320)
var writer = new LameMP3FileWriter(fileName, supportedWaveFormat, _userSettings.Bitrate);

context : https://github.com/jwallet/spy-spotify/blob/1.8/EspionSpotify/Recorder.cs#L224

code used on 1.1.6 (after upgrading to NAudio 1.10.0)

// stream : Stream of FileStream.Create("..\fileName.mp3", FileMode.Create, FileAccess.Write, FileShare.Read)
// supportedWaveFormat : Naudio.WaveFormat (48kHz sample rate, 2 channels)
// _userSettings.Bitrate : LAMEPreset (one of those 4 : ABR_128, ABR_160, ABR_256, ABR_320)
var writer = new LameMP3FileWriter(stream, supportedWaveFormat, _userSettings.Bitrate);

context : https://github.com/jwallet/spy-spotify/blob/master/EspionSpotify/Recorder.cs#L249
used like so : https://github.com/jwallet/spy-spotify/blob/master/EspionSpotify/Recorder.cs#L201:L214

I know that's Average Bit Rate but it should be around the choosen rate, but using the latest version it encodes 1 song every 5 tracks at 35kbps which gives me a track with a length of 18minutes instead of 4minutes, when I play the song from start to finish it stop at 4min/18min.

So downgrading the lib to 1.0.9 seems to help it for me, still using NAudio 1.10.0, bitrates are less random, it goes in a range of 150 to 170 for a 160ABR, it used to be more accurate so the media length was actually the right one.

I will continue debugging it, I will close if it comes from another place

So, it looks like it's indeed related to creating a the stream using FileStream.Create() instead of File.Create() like your lib does. I changed it to be able to set the FileAccess and FileShare of the stream, so other process can read it while this one is writing.

It's what NAudio does for it's WaveFileWriter, so I thought I could do the same thing with Lame lib, but it fails to keep a proper bit rate.

https://github.com/naudio/NAudio/blob/a2cc704b6cf9e1bbefd2e7fff14d5c5e19e8c2a0/NAudio/Wave/WaveOutputs/WaveFileWriter.cs#L112:L113

it also has random bitrate using
File.Open("x.mp3", FileMode.Create, FileAccess.Write, FileShare.ReadWrite))

(I keep looking)

image

All recorded in ABR_160

🔴 (red) : was recorded with File.Create(path) on 1.1.6
🔵 (blue) : was recorded with FileStream.Create(path, FileMode.Create, FileAccess.Write, FileShare.Read) on 1.1.6
🟡 (white-ish yellow): was recorded with File.Create(path) on 1.0.9

underlined time is to show time difference; same color, same track:
🟢 (green): 3:15 is the right one
🟡 (yellow): 4:15 is the right one

I did some work on the ABR/VBR initialisation over the last few releases which is probably why you're seeing more consistent results when using the ABR encodings.

For the rest, what you're seeing is the result of writing a provisional VBR header and not updating it after compression is complete.

Long story short: don't use write-only streams.

Longer version:

The LAME encoder generates frames which LameMP3FileWriter writes to the stream. If metadata is present the ID3 data is written first, followed by an optional (and provisional) VBR metadata frame, then the actual encoded frames. After all of the frames have been encoded and flushed to disk the VBR metadata frame is updated with the right information. In order to do this we need to locate the metadata frame... which means we need to read the data back from the file.

At the moment the code requires seek, read and write access to the file. As fun as it might be to try to work around the problem by guessing at the size of the ID3 data block without being able to read it, it's far simpler to just allow read access to the file you're creating. We already have to seek in the file and update the frame, just let it have its way.

make sense, thanks

I solved it by recording to wav with WaveFileWriter and then converting to mp3 with MediaFoundationEncoder

@barrien2 in my case, @Corey-M is right just use FileAccess with Read and Write

// I create my own stream and use LameMP3FileWriter with a stream instead of a fileName to be able to share the file
// otherwise I get IO Exceptions everywhere since I have a service that reads it at the same time while it's being recorded
using (var mediaFileStream = new FileStream("create-new.mp3", FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite))
{
     var supportedWaveFormat = GetWaveFormatMP3Supported(_waveIn.waveFormat); // fallback to mp3 Stereo 48khz if above
     var bitRates = GetUserBitRates(); // e.g.: LAMEPreset.ABR_160
     using (var mediaWriter = new LameMP3FileWriter(mediaFileStream, supportedWaveFormat, bitRates)
     {
          // tempReader is my wave reader above, it's what was recorded using WASAPI
          await tempReader.CopyToAsync(mediaWriter, 81920, _cancellationTokenSource.Token);
     }
}