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

24/96 flac content #30

Closed
mehrvarz opened this issue Aug 16, 2018 · 21 comments
Closed

24/96 flac content #30

mehrvarz opened this issue Aug 16, 2018 · 21 comments

Comments

@mehrvarz
Copy link

Are 24 bps 96khz flac files supported in general? Has anyone played back such content via mewkiz/flac? With the help of mewkiz/flac and gordonklaus/portaudio I am able to play back standard 16/44.1 flac files (some perfectly / some almost perfectly, see #29 ). However, I'm not yet able to play back 24/96 files in the same way. The problem is most likely in my code. But it could also be an issue in either mewkiz/flac or gordonklaus/portaudio. This is why I would like to ask if anybody has managed to play back 24/96 flac content from their Go app. (I can play back 24/96 flac files via VLC to my USB DAC in bitperfect fashion. Just not via my Go app, yet.) Thank you.

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

Hi @mehrvarz!

We just tried playing a FLAC file with 24 bps and 96kHz and it worked without glitches. Would you please try playing your FLAC file with the following snippet:

package main

import (
	"flag"
	"fmt"
	"log"
	"os"
	"time"

	"github.com/faiface/beep"
	"github.com/faiface/beep/flac"
	"github.com/faiface/beep/speaker"
)

func main() {
	flag.Parse()
	if flag.NArg() != 1 {
		fmt.Fprintf(os.Stderr, "Usage: blip FILE\n")
		os.Exit(1)
	}
	fr, err := os.Open(flag.Arg(0))
	if err != nil {
		log.Fatalf("unable to open file; %v", err)
	}
	s, format, err := flac.Decode(fr)
	speaker.Init(format.SampleRate, format.SampleRate.N(time.Second/10))
	done := make(chan struct{})
	f := func() {
		close(done)
	}
	notify := beep.Callback(f)
	speaker.Play(beep.Seq(s, notify))
	select {
	case <-done:
		break
	}
}

Note: github.com/faiface/beep/flac uses github.com/mewkiz/flac under the hood.

@mehrvarz
Copy link
Author

Cool. Unfortunately I get...

../gopath/pkg/mod/github.com/hajimehoshi/oto@v0.1.2/player.go:25:18: undefined: player

hajimehoshi/oto is referenced from faiface/beep/speaker.

Before that I tried using mewkiz/flac/cmd/flac2wav to convert some of my 24/96 flac files to wav format. But when I try to play back these wav files via VLC, I get the same scrambled audio noise that I get when I play them back directly via gordonklaus/portaudio.

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

Before that I tried using mewkiz/flac/cmd/flac2wav to convert some of my 24/96 flac files to wav format. But when I try to play back these wav files via VLC, I get the same scrambled audio noise that I get when I play them back directly via gordonklaus/portaudio.

Hmm. Would you mind uploading a sample FLAC file that exhibits this issue? Definitely sounds like you uncovered something incorrect in the flac library.

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

Cool. Unfortunately I get...

../gopath/pkg/mod/github.com/hajimehoshi/oto@v0.1.2/player.go:25:18: undefined: player

Try running

go get -v -u github.com/mewspring/blip

And use the blip tool to play the FLAC file. Does that work?

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

I can confirm that it is lagging with github.com/mesilliac/pulse-simple for the sample you uploaded Sample_BeeMoved_96kHz24bit.flac.

However, playing the sample file Sample_BeeMoved_96kHz24bit.flac with github.com/faiface/beep works perfectly on my laptop without any glitches.

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

Oh, and thanks for the uploaded FLAC files!

@mehrvarz
Copy link
Author

mehrvarz commented Aug 16, 2018

Yes github.com/mewspring/blip does work well and it seems to be able to play back all my flac content correctly (Edit: see below). I tested several 24/96 files and also some 16/44.1 files with blockSize=5.

mewkiz/flac/cmd/flac2wav still does not work for 24/96 content. Does mewspring/blip depend solely on mewkiz/flac? Or does it work differently?

@mehrvarz
Copy link
Author

I just notice that mewspring/blip does not seem to make use of mewkiz/flac. What do I need to do to get the working flac decoder?

@mehrvarz
Copy link
Author

So mewspring/blip does make use of mewkiz/flac internally. But I found some essential code in faiface/beep/flac/decode.go in regard to several bps cases. Should this not be part of the decoder package? But still very useful. Thank you for now.

@mehrvarz
Copy link
Author

faiface/beep/flac/decode.go seems to be converting 24bit down to 16bit by throwing out bits before playback. A rather simplified solution. Do you agree?

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

faiface/beep/flac/decode.go seems to be converting 24bit down to 16bit by throwing out bits before playback. A rather simplified solution. Do you agree?

If that's the case it's definitely a bug and should be fixed. Where did you see the truncation to 16 bit?

As far as I can tell from https://github.com/faiface/beep/blob/master/flac/decode.go#L116 the 24-bit samples are converted to int32 before being converted to a floating-point value.

	case bps == 24 && nchannels >= 2:
		for i := 0; i < n; i++ {
			d.buf[i][0] = float64(int32(frame.Subframes[0].Samples[i])) / (1<<23 - 1)
			d.buf[i][1] = float64(int32(frame.Subframes[1].Samples[i])) / (1<<23 - 1)
		}

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

As of rev da5a5e9 go-audio/wav is used instead of the WAV encoder provided by the Azul3D engine. The rationale being that go-audio/wav seem to be maintained and under active development.

Also, with the switch flac2wav is able to convert Sample_BeeMoved_96kHz24bit.flac to a WAV file that plays back perfectly. Please give it a try and report back :)

@mehrvarz
Copy link
Author

The code two posts up, is this your code? Do you happen to know what "divide by (1<<23 - 1)" is meant to do? The intent appears to be to divide by 0x800000 (not 0x7fffff). In other words to do: (val >> 24). Aren't bits being thrown out here? Don't you think that doing 24bit should involve some bit packing? As in 4 x 24 = 3 x int32 ?

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

The code two posts up, is this your code? Do you happen to know what "divide by (1<<23 - 1)" is meant to do? The intent appears to be to divide by 0x800000 (not 0x7fffff). In other words to do: (val >> 24). Aren't bits being thrown out here? Don't you think that doing 24bit should involve some bit packing? As in 4 x 24 = 3 x int32 ?

You spotted an off-by-one, good catch :) Pull request sent to beep: faiface/beep#38

@mewmew
Copy link
Member

mewmew commented Aug 16, 2018

As for the intuition behind it is that we wish to turn the integer value into a floating-point value in the range [-1.0, 1.0]. Therefore we wish to divide an 8-bit value [-128, 127] by the middle point, i.e. 1<<(8-1) or 128.0.

For an implementation of this, see https://godoc.org/zikichombo.org/sound/sample#ToFloat

func ToFloat(d int64, nBits int) float64 {
	s := float64(int64(1 << uint(nBits-1)))
	return float64(d) / s
}

@mewmew
Copy link
Member

mewmew commented Aug 17, 2018

Hi @mehrvarz! Can this be closed as well? Or is there something else that needs to be fixed.

@mehrvarz
Copy link
Author

mehrvarz commented Aug 17, 2018

I have something to ask about this still. I will come back soon in this regard.

@mewmew
Copy link
Member

mewmew commented Aug 17, 2018

I have something to ask about this still. I will came back soon in this regard.

Sure.

@mehrvarz
Copy link
Author

All issues specific to 24/96 content have been solved. I see occasional performance issues running the code on a Raspberry Pi. Need to investigate this further. This is not specific to 24/96 content, so we should close this thread now. Thank you very much for your help.

@mewmew
Copy link
Member

mewmew commented Aug 19, 2018

All issues specific to 24/96 content have been solved. I see occasional performance issues running the code on a Raspberry Pi. Need to investigate this further. This is not specific to 24/96 content, so we should close this thread now. Thank you very much for your help.

Great to hear! Glad to worked out with 24/96 samples. Just open up discussions when we need to troubleshoot something else. And thanks for helping to spot bugs and stress-testing the implementation :) You're probably the first user to put serious pressure on the library that has made themselves aware to us by reporting issues. Besides the pull requests sent of course, but we have very little knowledge what workloads they've tried to spot those issues. Oh, and the fuzzing (#10) as well, which was great to stress test the flac library as well.

@mewmew mewmew closed this as completed Aug 19, 2018
@cswank cswank mentioned this issue Feb 5, 2021
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