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

speaker.Play() seems to introduce a memory leak. #179

Closed
LuSrackhall opened this issue Aug 31, 2024 · 21 comments · Fixed by #138
Closed

speaker.Play() seems to introduce a memory leak. #179

LuSrackhall opened this issue Aug 31, 2024 · 21 comments · Fixed by #138
Labels
bug Something isn't working

Comments

@LuSrackhall
Copy link

For example, when playing .ogg files, the corresponding memory usage is not released after normal playback.

It seems that the speaker does not provide an effective method to release the memory.

@MarkKremer
Copy link
Contributor

MarkKremer commented Aug 31, 2024

Hello 👋

edit: I wasn't sure if you're talking about just the speaker & driver structs not being released or about the streamed sample buffers or something related not being freed causing a repeated memory leak. The comment below is about the latter.

Could you share some information about the issue?

  • What platform are you running on?
  • How are you measuring the memory leak?
  • Does using another decoder still cause the memory leak?

Does the following program produce the memory leak for you? I've also added some debug information. Could you post the output?

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"time"

	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	f, err := os.Open("path/to/your.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/30))
	if err != nil {
		return
	}

	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	speaker.PlayAndWait(streamer)
}

@LuSrackhall
Copy link
Author

LuSrackhall commented Aug 31, 2024

Thank you for your example, it helped me quickly pinpoint the issue in my code.

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"time"

	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	for i := 0; i < 300; i++ {
		time.Sleep(100 * time.Millisecond)
		go play()
	}

	for {

	}
}

func play() {
	f, err := os.Open("path/to/your.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/30))
	// I did not make a wrong judgment on this position because my business requires playing multiple audio simultaneously. The location of the leak should have occurred due to my incorrect use here.
	// Therefore, this issue should be able to be closed now.
	// if err != nil {
	// 	return
	// }

	speaker.PlayAndWait(streamer)
}

I will close this issue because it seems to be related to another topic.

New topic: How can beep safely and concurrently play audio?

I would greatly appreciate it if you could answer before I close this issue!

Even if you don’t answer, I will still close this issue.

Afterwards, I may continue to explore how to achieve my own business needs or start a new issue with the actual new topic.

@MarkKremer
Copy link
Contributor

MarkKremer commented Aug 31, 2024

So in short there are two things you need to take care of:

  • speaker.Init() can only be called once during your whole program. If you need to play multiple streamers of different sample rates, initialize the speaker with a sample rate that you find suitable and use the Resampler to change the sample rate of your audio to the speaker's sample rate. (https://github.com/gopxl/beep/wiki/Hello,-Beep!#dealing-with-different-sample-rates)
  • Multiple sounds can be played at the same time, but a single goroutine pulls the audio samples from the streamers to mix them together. The streamers should not be changed while that is happening. You can use speaker.Lock() and speaker.Unlock() to prevent the speaker from calling the streamer and causing concurrency issues. You don't need to do this for calling functions on the speaker directly (like Play()). Those do the locking for you. (https://github.com/gopxl/beep/wiki/Composing-and-controlling)

Also, PlayAndWait is more of a convenience helper. If you want to play multiple sounds, just use Play and use another way to detect that the streamers are done playing (the Seq example here). That way you don't need to start multiple goroutines to start playing which doesn't actually do the work concurrently).

Hope this helps! If you have any questions, I have more time tomorrow.

I'm also taking notes on what parts of the docs could use some attention. So your comment helped me.

@LuSrackhall
Copy link
Author

LuSrackhall commented Sep 1, 2024

Thank you for taking the time to respond and help me.

Of course, the following is still my error example, but I really don’t know how to handle it correctly to meet my needs.

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"sync"
	"time"

	"github.com/gopxl/beep/v2"
	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	f, err := os.Open("path/to/your.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}
	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}
	mixer := &beep.Mixer{}

	// mixer.Add(streamer)
	// mixer.Add(streamer)
	// mixer.Add(beep.Resample(4, format.SampleRate, sr*2, streamer))
	// mixer.Add(beep.Resample(4, format.SampleRate, sr*2, streamer))
	done := make(chan bool)
	mixer.Add(beep.Seq(streamer, beep.Callback(func() {
		done <- true
	})))

	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	speaker.Play(beep.Seq(mixer, beep.Callback(func() {
		done <- true
	})))

	var mu sync.Mutex
	for i := 0; i < 3; i++ {
		time.Sleep(3000 * time.Millisecond)

		mu.Lock()
		// mixer.Add(streamer)
		mixer.Add(beep.Resample(4, format.SampleRate, sr*2^beep.SampleRate(i+1), streamer))
		mu.Unlock()
	}

	for {
		select {
		case <-done:
			fmt.Println("end")
		}
	}
}

It might be related to the first point you mentioned, but even when I use the same audio file, mixer.Add() doubles the speed. I tried using the fix from the documentation to change this behavior, but it didn’t achieve the expected result.

Maybe my approach has deviated from the correct usage of beep, but I do have limited knowledge about audio-related topics. Beep has indeed provided me with great help in this regard, but due to my own issues, I can’t use beep effectively to meet my business goals.

My business requirements are actually very simple:

  • Concurrently play multiple audio files
    • For example, first play an audio file, and in the middle of playing, play a second audio file (the first audio file should continue playing at its original progress), and so on, the third, fourth (even nth). -- <Currently stuck here because the previous solution without checking err = speaker.Init() seemed to solve the issue, but this incorrect usage of beep inevitably caused memory leaks>
    • Of course, each audio file should be able to play correctly until the end.
    • And, for each audio file that is playing, when I need it to end, actively end it (or, in other words, only play a portion of it).
      • It seems that using "Seek() and monitoring Position()" , and calling ctrl.Paused = true can solve this.
      • Although buffer.Streamer(start, end) can also be used, I don’t want to introduce a buffer.
    • Lastly, the memory used for playing audio can be completely released.

you don’t need to worry about it, as my business is just some of my own small toys.

Please prioritize your own work!

Lastly, please forgive my disturbance!

@LuSrackhall
Copy link
Author

LuSrackhall commented Sep 1, 2024

My previous reply was too hasty.

The issue I’m currently facing seems to be how to change the sample rate to maintain the original speed when playing multiple audio files, as they always seem to speed up automatically.

I haven’t figured out where the variable is exactly. It seems to be the number of calls, but if my number of calls is dynamic, how should I handle it?

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"time"

	"github.com/gopxl/beep/v2"
	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	f, err := os.Open("path/to/your.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}
	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}

	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	done := make(chan bool)

	go (func() {
		for i := 0; i < 10; i++ {
			time.Sleep(3000 * time.Millisecond)
			// Adding them seems to have improved things, but it seems to involve some luck, as I’m currently a bit confused.
			// speaker.Lock()
			// streamer.Seek(0)
			// speaker.Unlock()
			
			// // i<n
			// // When n = 1, i.e., only playing once, everything is normal.
			// // When n = 2, the playback speed increases slightly.
			// // As the number of calls continues to increase, the playback speed of all audio (including the currently playing audio) will increase.
			// speaker.Play(beep.Seq(streamer, beep.Callback(func() {
			// 	done <- true
			// })))

			// I tried to use beep.Resample to change this phenomenon, but it failed.
			speaker.Play(beep.Seq(beep.Resample(4, format.SampleRate, sr*2^beep.SampleRate(i), streamer), beep.Callback(func() {
				done <- true
			})))

		}
	})()

	for {
		select {
		case <-done:
			fmt.Println("end")
		}
	}
}

@MarkKremer
Copy link
Contributor

It's only possible to use a file or streamer once. If the same streamer is played twice, the speaker will first pull from streamer, then pull from streamer again and mix those sounds together. The result is that you go twice as fast through streamer, and possibly some glitchy sound artifacts.

To solve this you would either have to use the buffer like you mentioned, or open the file multiple times and a streamer for each of them (multiple streamers for one file doesn't work because they will both progress the position in the file). There's also the Dup node which is another kind of buffer but you would need multiple copies of them for your use case so then a normal buffer would be better.

@LuSrackhall
Copy link
Author

LuSrackhall commented Sep 1, 2024

Thank you for taking the time to respond and help me.

I only called speaker.Init once, using a fixed sample rate.

However, multiple calls to play() seem to change the sample rate initialized by speaker.Init, which leaves me at a loss (because there is more than one variable, and it is even related to the number of calls).

The above is just me using the same audio file with the same sample rate for concurrent playback.

If I introduce multiple audio files with different sample rates for concurrent playback, it seems to be more complicated.

In fact, in this case, if a program could have multiple speaker instances within its lifecycle, it could be easily solved.

I don’t understand the implementation principle, my idea might be a joke.

But personally, I think such usage can indeed reduce a lot of confusion.

After all, the playback speed inexplicably accelerating automatically makes it difficult for a simple user like me, who doesn’t understand the principle, to easily get started, and even harder to use it well after getting started.

I will continue to try the solution of opening the file multiple times based on your latest reply, and also consider the buffer. In short, I have figured out the specific reason for this phenomenon from your latest reply. Thank you for your help. If I had continued on my own today, I might not have discovered it.

Thank you.

@LuSrackhall
Copy link
Author

Hello!!

I’m sorry to bother you again, but this time I can reproduce the memory leak. I think it is still related to concurrent playback. Therefore, I will reopen this issue.

After many attempts, I found that the amount of leakage is positively correlated with the frequency of concurrent calls.

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"time"

	"github.com/gopxl/beep/v2"
	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}

	f.Close()
	streamer.Close()

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}

	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	// TIPS: The memory leak exists, and this example can reproduce the issue on my computer.
	for i := 0; i < 100; i++ {
		// FIXME:
		// If no delay is added here, or if multiple audio files are played concurrently at the maximum frequency, the memory leak phenomenon can be observed directly.
		// time.Sleep(3000 * time.Millisecond)// Of course, the amount of leakage is related to the frequency of our calls. If you uncomment this and add a certain delay, the amount of memory leakage after all playback is completed will be much smaller than in the case of no delay at all.

		go Play(i)
	}

	for {
	}

}

func Play(count int) {
	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}
	defer streamer.Close()

	done := make(chan bool)

	fmt.Println("start number  ", count)
	speaker.Play(beep.Seq(streamer, beep.Callback(func() {
		done <- true
	})))

	<-done
	fmt.Println("end number  ", count)
}

@LuSrackhall LuSrackhall reopened this Sep 2, 2024
@LuSrackhall
Copy link
Author

I now feel that maybe I did something wrong in my example!!!

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"time"

	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}

	f.Close()
	streamer.Close()

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}

	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	// TIPS: The memory leak exists, and this example can reproduce the issue on my computer.
	for i := 0; i < 1000; i++ {
		// FIXME:
		// If no delay is added here, or if multiple audio files are played concurrently at the maximum frequency, the memory leak phenomenon can be observed directly.
		// time.Sleep(3000 * time.Millisecond)// Of course, the amount of leakage is related to the frequency of our calls. If you uncomment this and add a certain delay, the amount of memory leakage after all playback is completed will be much smaller than in the case of no delay at all.

		go Play(i)
	}

	for {
	}

}

func Play(count int) {

	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer (func() {
		err := f.Close()
		if err != nil {
			fmt.Println("close file error", err)
			// log.Fatal(err)
		}
	})()

	// If vorbis.Decode() is not used here, re-observing the memory reveals that there is still a leakage.
	// streamer, format, err := vorbis.Decode(f)
	// fmt.Println(format.SampleRate)
	// if err != nil {
	// 	log.Fatal(err)
	// }
	// defer streamer.Close()

	done := make(chan bool)

	fmt.Println("start number  ", count)

	// If speaker.Play() is not used here, the simulation ends after the same duration as the audio. (My test audio is 30 seconds, so I used a 30-second delay.) Re-observing the memory reveals that there is still a leakage.
	// speaker.Play(beep.Seq(streamer, beep.Callback(func() {
	// 	done <- true
	// })))
	go (func() {
		time.Sleep(30000 * time.Millisecond)
		done <- true
	})()

	<-done
	fmt.Println("end number  ", count)
}

@LuSrackhall
Copy link
Author

LuSrackhall commented Sep 2, 2024

It should be my own problem, I’m very sorry for bothering everyone!

I will close the issue because it seems to be caused by opening too many goroutines and my shallow understanding of Golang, which led to improper resource management. It has nothing to do with the beep library.

package main

import (
	"fmt"
	"log"
	"os"
	"runtime"
	"time"

	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"
)

func PrintMemUsage() {
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
	fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
	return b / 1024 / 1024
}

func main() {
	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}

	streamer, format, err := vorbis.Decode(f)
	fmt.Println(format.SampleRate)
	if err != nil {
		log.Fatal(err)
	}

	f.Close()
	streamer.Close()

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}

	go func() {
		for {
			PrintMemUsage()
			time.Sleep(5 * time.Second)
		}
	}()

	// TIPS: The memory leak exists, and this example can reproduce the issue on my computer.
	for i := 0; i < 100; i++ {
		// FIXME:
		// If no delay is added here, or if multiple audio files are played concurrently at the maximum frequency, the memory leak phenomenon can be observed directly.
		// time.Sleep(3000 * time.Millisecond)// Of course, the amount of leakage is related to the frequency of our calls. If you uncomment this and add a certain delay, the amount of memory leakage after all playback is completed will be much smaller than in the case of no delay at all.

		go Play(i)
	}

	for {
	}

}

// I will close the issue because it seems to be caused by opening too many goroutines and my shallow understanding of Golang, which led to improper resource management. It has nothing to do with the beep library.
func Play(count int) {
	// If os.Open() is not used here, re-observing the memory reveals that there is still a leakage.
	// f, err := os.Open("sounds/sound.ogg")
	// if err != nil {
	// 	log.Fatal(err)
	// }
	// defer (func() {
	// 	err := f.Close()
	// 	if err != nil {
	// 		fmt.Println("close file error", err)
	// 		// log.Fatal(err)
	// 	}
	// })()

	// If vorbis.Decode() is not used here, re-observing the memory reveals that there is still a leakage.
	// streamer, format, err := vorbis.Decode(f)
	// fmt.Println(format.SampleRate)
	// if err != nil {
	// 	log.Fatal(err)
	// }
	// defer streamer.Close()

	done := make(chan bool)

	fmt.Println("start number  ", count)

	// If speaker.Play() is not used here, the simulation ends after the same duration as the audio. (My test audio is 30 seconds, so I used a 30-second delay.) Re-observing the memory reveals that there is still a leakage.
	// speaker.Play(beep.Seq(streamer, beep.Callback(func() {
	// 	done <- true
	// })))
	go (func() {
		// time.Sleep(30000 * time.Millisecond)
		done <- true
	})()

	<-done
	fmt.Println("end number  ", count)
}

@MarkKremer
Copy link
Contributor

MarkKremer commented Sep 2, 2024

Hey, don't worry about it :). I didn't become a maintainer to sit on a hill and kick everyone off.

Just note that anything (variables, streamers, goroutines, ...) that you instantiate will use memory until they are released. If you create 100 of something, off course that will use some memory. Also you can't have a program that uses 0 memory. Only if the objects are not needed anymore and they aren't cleaned up, then it becomes what's called a memory leak.

To be honest, I would remove the memory statistics from your program and focus on making it work. I think it's a distraction at this point. Feel free to join our Discord (see badge link at the top of the README) if you have more questions. :) or use this or new issues, that's fine as well.

@MarkKremer
Copy link
Contributor

Also, you questions are valuable to me so I can discover where Beep's rough edges are & I can improve it.

@LuSrackhall
Copy link
Author

Hello!

Everyone's time is precious, and I admire your dedication!

Go seems to provide a tool called pprof, but unfortunately, I haven't learned how to use it yet.

Therefore, my example may not be very professional, but I currently don't have the energy to learn more.

So, I still throw out my question. Of course, I still hope you prioritize your own work, as my example may not be sufficient to illustrate the problem.

Because I still think there might be a leak in speaker.Play(), here are my latest observations.

My Go version is 1.22.0, and my operating system is Win10.

package main

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

	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"

	"net/http"
	_ "net/http/pprof"
)

// func PrintMemUsage() {
// 	var m runtime.MemStats
// 	runtime.ReadMemStats(&m)
// 	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
// 	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
// 	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
// 	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
// 	fmt.Printf("\tNumGC = %v\n", m.NumGC)
// 	fmt.Println("Number of goroutines:", runtime.NumGoroutine())
// }

// func bToMb(b uint64) uint64 {
// 	return b / 1024 / 1024
// }

func main() {

	go func() {
		log.Println(http.ListenAndServe("localhost:6060", nil))
	}()

	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}

	streamer, format, err := vorbis.Decode(f)
	if err != nil {
		log.Fatal(err)
	}

	f.Close()
	streamer.Close()

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}
	/*
	 TIPS:
	 Please excuse me for canceling the related memory monitoring printout, as I have now found it to be inaccurate.
	 Using the Task Manager on my Windows 10 system, I can observe the memory release without using `speaker.Play()`.
	*/
	// go func() {
	// 	for {
	// 		PrintMemUsage()
	// 		time.Sleep(5 * time.Second)
	// 	}
	// }()

	{
		// TIPS: I will reduce the number of goroutines used for observation.
		//       > The more goroutines, the more necessary memory fragmentation, so please ignore this part and only observe whether the largest block of memory is released each time.
		//       > * If `speaker.Play()` is not used, the release process can be clearly observed in the Task Manager provided by the operating system.
		//       > * But if `speaker.Play()` is used, this process cannot be observed, and the largest block of memory seems to remain in the runtime forever.

		for i := 0; i < 100; i++ {
			go Play(i)
		}
	}

	done := make(chan struct{})
	select {
	case <-done:
		return
	}

}

func Play(count int) {
	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer (func() {
		err := f.Close()
		if err != nil {
			// TIPS:
			/*
				This comment is added because the `streamer.Close()` operation below has already closed this file descriptor.
				I don’t want it to print a message about closing it again, as that would affect our observation.
			*/
			// fmt.Println("close file error", err)
			// log.Fatal(err)
		}
	})()

	streamer, _, err := vorbis.Decode(f)
	if err != nil {
		log.Fatal(err)
	}
	defer (func() {
		err := streamer.Close()
		if err != nil {
			fmt.Println("close streamer error", err)
		}
	})()

	done := make(chan struct{})
	// done := make(chan bool)
	defer close(done)

	// fmt.Println("start number  ", count)

	/*
		If speaker.Play() is not used here, the simulation ends after the same duration as the audio.
		> My test audio is 30 seconds, so I used a 30-second delay.
	*/
	{
		{
			// TIPS: After the time ends, the memory is released
			// 	    * this can be observed in the Task Manager provided by the operating system.
			go (func() {
				time.Sleep(30000 * time.Millisecond)
				done <- struct{}{}
				// done <- true
			})()
		}
		{
			// FIXME: After the time ends, the memory still remains in the program
			//            * this can be observed in the Task Manager provided by the operating system.
			// speaker.Play(beep.Seq(streamer, beep.Callback(func() {
			// 	done <- struct{}{}
			// 	// done <- true
			// })))
		}
	}

	<-done
	// fmt.Println("end number  ", count)
}

Of course, it is not excluded that, like the above examples, this is still caused by resources within the goroutine not being properly garbage collected (GC). --> after using speaker.Play(), more memory resources are allocated, resulting in more residuals.

Currently,The current issue regarding the necessary residuals of goroutines, apart from using a goroutine pool to avoid creating too many goroutines as a temporary solution (for scenarios where short segments are played for a short time), I have not found an effective way to properly release the resources used by finished goroutines. (In fact, I believe these should be correctly reclaimed by Go’s built-in runtime GC. However, based on the observation that the more resources a goroutine task uses, the more residuals there are, this does not meet the expectations of GC.)

Let’s get back to our topic.

The final remaining large memory block may be caused by the cache of slices in beep not being successfully released.

Although there is a small probability that it is related to the necessary residuals of goroutines, large resource occupancies like file descriptors can be correctly released, so beep should be able to do it as well.

@LuSrackhall LuSrackhall reopened this Sep 4, 2024
@MarkKremer
Copy link
Contributor

MarkKremer commented Sep 4, 2024

You could save me some time by testing your code with my old PR #138. I have a suspicion about some code that I happened to fix in that branch. Hopefully, it's still compatible.

@LuSrackhall
Copy link
Author

Hello!

I am very happy to do this test!

This is my test code. When I used the new Mixer in the PR, the memory leak problem was solved. Now beep can correctly release the memory occupied after playback ends.

package main

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

	"github.com/gopxl/beep/v2"
	"github.com/gopxl/beep/v2/speaker"
	"github.com/gopxl/beep/v2/vorbis"

	"net/http"
	_ "net/http/pprof"
)

// func PrintMemUsage() {
// 	var m runtime.MemStats
// 	runtime.ReadMemStats(&m)
// 	// For info on each, see: https://golang.org/pkg/runtime/#MemStats
// 	fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
// 	fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
// 	fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
// 	fmt.Printf("\tNumGC = %v\n", m.NumGC)
// 	fmt.Println("Number of goroutines:", runtime.NumGoroutine())
// }

// func bToMb(b uint64) uint64 {
// 	return b / 1024 / 1024
// }

func main() {

	go func() {
		log.Println(http.ListenAndServe("localhost:6060", nil))
	}()

	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}

	streamer, format, err := vorbis.Decode(f)
	if err != nil {
		log.Fatal(err)
	}

	f.Close()
	streamer.Close()

	sr := format.SampleRate
	err = speaker.Init(sr, sr.N(time.Second/36))
	if err != nil {
		return
	}
	/*
	 TIPS:
	 Please excuse me for canceling the related memory monitoring printout, as I have now found it to be inaccurate.
	 Using the Task Manager on my Windows 10 system, I can observe the memory release without using `speaker.Play()`.
	*/
	// go func() {
	// 	for {
	// 		PrintMemUsage()
	// 		time.Sleep(5 * time.Second)
	// 	}
	// }()

	mixer := &beep.Mixer{}
	speaker.Play(mixer)

	{
		// TIPS: I will reduce the number of goroutines used for observation.
		//       > The more goroutines, the more necessary memory fragmentation, so please ignore this part and only observe whether the largest block of memory is released each time.
		//       > * If `speaker.Play()` is not used, the release process can be clearly observed in the Task Manager provided by the operating system.
		//       > * But if `speaker.Play()` is used, this process cannot be observed, and the largest block of memory seems to remain in the runtime forever.

		for i := 0; i < 100; i++ {
			// go Play(i)
			go Play(i, mixer)
		}
	}

	done := make(chan struct{})
	select {
	case <-done:
		return
	}

}

// func Play(count int) {
func Play(count int, mixer *beep.Mixer) {
	f, err := os.Open("sounds/sound.ogg")
	if err != nil {
		log.Fatal(err)
	}
	defer (func() {
		err := f.Close()
		if err != nil {
			// TIPS:
			/*
				This comment is added because the `streamer.Close()` operation below has already closed this file descriptor.
				I don’t want it to print a message about closing it again, as that would affect our observation.
			*/
			// fmt.Println("close file error", err)
			// log.Fatal(err)
		}
	})()

	streamer, _, err := vorbis.Decode(f)
	if err != nil {
		log.Fatal(err)
	}
	defer (func() {
		err := streamer.Close()
		if err != nil {
			fmt.Println("close streamer error", err)
		}
	})()

	done := make(chan struct{})
	// done := make(chan bool)
	defer close(done)

	// fmt.Println("start number  ", count)

	/*
		If speaker.Play() is not used here, the simulation ends after the same duration as the audio.
		> My test audio is 30 seconds, so I used a 30-second delay.
	*/
	{
		{
			// TIPS: After the time ends, the memory is released
			// 			 * this can be observed in the Task Manager provided by the operating system.
			// go (func() {
			// 	time.Sleep(30000 * time.Millisecond)
			// 	done <- struct{}{}
			// 	// done <- true
			// })()
		}
		{
			// FIXME: After the time ends, the memory still remains in the program
			//        * this can be observed in the Task Manager provided by the operating system.
			// speaker.Play(beep.Seq(streamer, beep.Callback(func() {
			// 	done <- struct{}{}
			// 	// done <- true
			// })))
			mixer.Add(beep.Seq(streamer, beep.Callback(func() {
				done <- struct{}{}
			})))
		}
	}

	<-done
	fmt.Println("end number  ", count)
}

Your PR has completely solved the current problem. It is very effective! Thank you!

As for when to close the current issue, I think it should be up to you to decide, perhaps when this old PR is merged into the main branch!

@MarkKremer
Copy link
Contributor

MarkKremer commented Sep 5, 2024

Glad to hear it. Yes, I'll close this issue when the PR is merged. I'll have to re-review what's in that PR, so should be in a couple days to weeks time. edit: the PR technically breaks backwards-compatibility. I'll have to see what I do with that.

If you're curious about the problem. I suspect this was the cause:

last := len(m.streamers) - 1
m.streamers[si] = m.streamers[last]
m.streamers[last] = nil // the previous version of the mixer didn't do this.
m.streamers = m.streamers[:last]

This is the code for deleting a drained streamer at index si. The slice is shrunk, but if the pointer to the streamer falls outside the range of the slice, but still exists, the garbage collector will not collect it.

For your code: note that the speaker uses a mixer itself, so you don't need to prepend a mixer to the speaker.

@MarkKremer MarkKremer added the bug Something isn't working label Sep 5, 2024
@LuSrackhall
Copy link
Author

iso格式week

@MarkKremer
Copy link
Contributor

Ah

The PR is pre-v2 release so it still references v1 in imports. Will be fixed before/during merging.

@MarkKremer
Copy link
Contributor

Updated the PR. It's for review from my co-maintainers now.

@MarkKremer
Copy link
Contributor

@LuSrackhall PR is merged and I made a release. Feel free to comment or make a new issue if there are any more issues! :)

@LuSrackhall
Copy link
Author

Great! Thank you very much for your hard work and contribution! I will continue to keep an eye on related issues during my usage!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants