Merge pull request #127 from gopxl/fix-flac-decode-eof

Fix flac.Decode handling of io.EOF

CHANGELOG.md

@@ -11,7 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Support for single channel ogg/vorbis ([#10](https://github.com/gopxl/beep/pull/10))
 
 ### Fixed
-- Fix FileSize for saving .wav ([#6](https://github.com/gopxl/beep/pull/6))
+- Fix `FileSize` for saving .wav ([#6](https://github.com/gopxl/beep/pull/6))
+- Fix `flac.Decode` handling of `io.EOF` ([#127](https://github.com/gopxl/beep/pull/127))
 
 ### Changed
 - Upgrade Go version to 1.21 ([#2](https://github.com/gopxl/beep/pull/2))

compositors_test.go

@@ -6,73 +6,17 @@ import (
 	"testing"
 
 	"github.com/gopxl/beep"
+	"github.com/gopxl/beep/internal/testtools"
 )
 
-// randomDataStreamer generates random samples of duration d and returns a StreamSeeker which streams
-// them and the data itself.
-func randomDataStreamer(numSamples int) (s beep.StreamSeeker, data [][2]float64) {
-	data = make([][2]float64, numSamples)
-	for i := range data {
-		data[i][0] = rand.Float64()*2 - 1
-		data[i][1] = rand.Float64()*2 - 1
-	}
-	return &dataStreamer{data, 0}, data
-}
-
-type dataStreamer struct {
-	data [][2]float64
-	pos  int
-}
-
-func (ds *dataStreamer) Stream(samples [][2]float64) (n int, ok bool) {
-	if ds.pos >= len(ds.data) {
-		return 0, false
-	}
-	n = copy(samples, ds.data[ds.pos:])
-	ds.pos += n
-	return n, true
-}
-
-func (ds *dataStreamer) Err() error {
-	return nil
-}
-
-func (ds *dataStreamer) Len() int {
-	return len(ds.data)
-}
-
-func (ds *dataStreamer) Position() int {
-	return ds.pos
-}
-
-func (ds *dataStreamer) Seek(p int) error {
-	ds.pos = p
-	return nil
-}
-
-// collect drains Streamer s and returns all of the samples it streamed.
-func collect(s beep.Streamer) [][2]float64 {
-	var (
-		result [][2]float64
-		buf    [479][2]float64
-	)
-	for {
-		n, ok := s.Stream(buf[:])
-		if !ok {
-			return result
-		}
-		result = append(result, buf[:n]...)
-	}
-}
-
 func TestTake(t *testing.T) {
 	for i := 0; i < 7; i++ {
 		total := rand.Intn(1e5) + 1e4
-		s, data := randomDataStreamer(total)
+		s, data := testtools.RandomDataStreamer(total)
 		take := rand.Intn(total)
 
 		want := data[:take]
-		got := collect(beep.Take(take, s))
+		got := testtools.Collect(beep.Take(take, s))
 
 		if !reflect.DeepEqual(want, got) {
 			t.Error("Take not working correctly")
@@ -83,13 +27,13 @@ func TestTake(t *testing.T) {
 func TestLoop(t *testing.T) {
 	for i := 0; i < 7; i++ {
 		for n := 0; n < 5; n++ {
-			s, data := randomDataStreamer(10)
+			s, data := testtools.RandomDataStreamer(10)
 
 			var want [][2]float64
 			for j := 0; j < n; j++ {
 				want = append(want, data...)
 			}
-			got := collect(beep.Loop(n, s))
+			got := testtools.Collect(beep.Loop(n, s))
 
 			if !reflect.DeepEqual(want, got) {
 				t.Error("Loop not working correctly")
@@ -105,15 +49,15 @@ func TestSeq(t *testing.T) {
 		data = make([][][2]float64, n)
 	)
 	for i := range s {
-		s[i], data[i] = randomDataStreamer(rand.Intn(1e5) + 1e4)
+		s[i], data[i] = testtools.RandomDataStreamer(rand.Intn(1e5) + 1e4)
 	}
 
 	var want [][2]float64
 	for _, d := range data {
 		want = append(want, d...)
 	}
 
-	got := collect(beep.Seq(s...))
+	got := testtools.Collect(beep.Seq(s...))
 
 	if !reflect.DeepEqual(want, got) {
 		t.Errorf("Seq not working properly")
@@ -127,7 +71,7 @@ func TestMix(t *testing.T) {
 		data = make([][][2]float64, n)
 	)
 	for i := range s {
-		s[i], data[i] = randomDataStreamer(rand.Intn(1e5) + 1e4)
+		s[i], data[i] = testtools.RandomDataStreamer(rand.Intn(1e5) + 1e4)
 	}
 
 	maxLen := 0
@@ -145,7 +89,7 @@ func TestMix(t *testing.T) {
 		}
 	}
 
-	got := collect(beep.Mix(s...))
+	got := testtools.Collect(beep.Mix(s...))
 
 	if !reflect.DeepEqual(want, got) {
 		t.Error("Mix not working correctly")
@@ -154,7 +98,7 @@ func TestMix(t *testing.T) {
 
 func TestDup(t *testing.T) {
 	for i := 0; i < 7; i++ {
-		s, data := randomDataStreamer(rand.Intn(1e5) + 1e4)
+		s, data := testtools.RandomDataStreamer(rand.Intn(1e5) + 1e4)
 		st, su := beep.Dup(s)
 
 		var tData, uData [][2]float64

flac/decode.go

@@ -4,9 +4,10 @@ import (
 	"fmt"
 	"io"
 
-	"github.com/gopxl/beep"
 	"github.com/mewkiz/flac"
 	"github.com/pkg/errors"
+
+	"github.com/gopxl/beep"
 )
 
 // Decode takes a Reader containing audio data in FLAC format and returns a StreamSeekCloser,
@@ -62,9 +63,12 @@ func (d *decoder) Stream(samples [][2]float64) (n int, ok bool) {
 		if j >= len(d.buf) {
 			// refill buffer.
 			if err := d.refill(); err != nil {
-				d.err = err
 				d.pos += n
-				return n, n > 0
+				if err == io.EOF {
+					return n, n > 0
+				}
+				d.err = err
+				return 0, false
 			}
 			j = 0
 		}

flac/decode_test.go

@@ -0,0 +1,39 @@
+package flac_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/gopxl/beep/flac"
+	"github.com/gopxl/beep/internal/testtools"
+)
+
+func TestDecoder_Stream(t *testing.T) {
+	f, err := os.Open(testtools.TestFilePath("valid_44100hz_22050_samples.flac"))
+	assert.NoError(t, err)
+	defer f.Close()
+
+	streamer, _, err := flac.Decode(f)
+	assert.NoError(t, err)
+
+	// Case 1: return ok with all requested samples
+	buf := testtools.CollectNum(22000, streamer)
+	assert.Lenf(t, buf, 22000, "streamer quit prematurely; expected %d samples, got %d", 22000, len(buf))
+	assert.NoError(t, streamer.Err())
+
+	buf = make([][2]float64, 512)
+
+	// Case 2: return ok with 0 < n < 512 samples
+	n, ok := streamer.Stream(buf[:])
+	assert.True(t, ok)
+	assert.Equal(t, 50, n)
+	assert.NoError(t, streamer.Err())
+
+	// Case 3: return !ok with n == 0
+	n, ok = streamer.Stream(buf[:])
+	assert.False(t, ok)
+	assert.Equal(t, 0, n)
+	assert.NoError(t, streamer.Err())
+}

interface.go

@@ -14,11 +14,11 @@ type Streamer interface {
 	// samples will be produced, it returns 0 and false. Stream must not touch any samples
 	// outside samples[:n].
 	//
-	// There are 3 valid return pattterns of the Stream method:
+	// There are 3 valid return patterns of the Stream method:
 	//
 	//   1. n == len(samples) && ok
 	//
-	// Stream streamed all of the requested samples. Cases 1, 2 and 3 may occur in the following
+	// Stream streamed all the requested samples. Cases 1, 2 and 3 may occur in the following
 	// calls.
 	//
 	//   2. 0 < n && n < len(samples) && ok

internal/testtools/sinks.go

@@ -0,0 +1,23 @@
+package testtools
+
+import "github.com/gopxl/beep"
+
+// Collect drains Streamer s and returns all the samples it streamed.
+func Collect(s beep.Streamer) [][2]float64 {
+	var (
+		result [][2]float64
+		buf    [479][2]float64
+	)
+	for {
+		n, ok := s.Stream(buf[:])
+		if !ok {
+			return result
+		}
+		result = append(result, buf[:n]...)
+	}
+}
+
+// CollectNum collects num samples from the streamer in chunks and returns them all.
+func CollectNum(num int, s beep.Streamer) [][2]float64 {
+	return Collect(beep.Take(num, s))
+}

internal/testtools/streamers.go

@@ -0,0 +1,48 @@
+package testtools
+
+import (
+	"math/rand"
+
+	"github.com/gopxl/beep"
+)
+
+// RandomDataStreamer generates numSamples random samples and returns a StreamSeeker to stream them.
+func RandomDataStreamer(numSamples int) (s beep.StreamSeeker, data [][2]float64) {
+	data = make([][2]float64, numSamples)
+	for i := range data {
+		data[i][0] = rand.Float64()*2 - 1
+		data[i][1] = rand.Float64()*2 - 1
+	}
+	return &dataStreamer{data, 0}, data
+}
+
+type dataStreamer struct {
+	data [][2]float64
+	pos  int
+}
+
+func (ds *dataStreamer) Stream(samples [][2]float64) (n int, ok bool) {
+	if ds.pos >= len(ds.data) {
+		return 0, false
+	}
+	n = copy(samples, ds.data[ds.pos:])
+	ds.pos += n
+	return n, true
+}
+
+func (ds *dataStreamer) Err() error {
+	return nil
+}
+
+func (ds *dataStreamer) Len() int {
+	return len(ds.data)
+}
+
+func (ds *dataStreamer) Position() int {
+	return ds.pos
+}
+
+func (ds *dataStreamer) Seek(p int) error {
+	ds.pos = p
+	return nil
+}

internal/testtools/testdata.go

@@ -0,0 +1,16 @@
+package testtools
+
+import (
+	"path/filepath"
+	"runtime"
+)
+
+var (
+	// See https://stackoverflow.com/a/38644571
+	_, callerPath, _, _ = runtime.Caller(0)
+	TestDataDir         = filepath.Join(filepath.Dir(callerPath), "../testdata")
+)
+
+func TestFilePath(path string) string {
+	return filepath.Join(TestDataDir, path)
+}

resample_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/gopxl/beep"
+	"github.com/gopxl/beep/internal/testtools"
 )
 
 func TestResample(t *testing.T) {
@@ -15,11 +16,11 @@ func TestResample(t *testing.T) {
 					continue // skip too expensive combinations
 				}
 
-				s, data := randomDataStreamer(numSamples)
+				s, data := testtools.RandomDataStreamer(numSamples)
 
 				want := resampleCorrect(3, old, new, data)
 
-				got := collect(beep.Resample(3, old, new, s))
+				got := testtools.Collect(beep.Resample(3, old, new, s))
 
 				if !reflect.DeepEqual(want, got) {
 					t.Fatal("Resample not working correctly")