Skip to content
This repository has been archived by the owner on Apr 2, 2023. It is now read-only.

Opened Context consumes 100% CPU #28

Open
dimonomid opened this issue Feb 15, 2019 · 13 comments
Open

Opened Context consumes 100% CPU #28

dimonomid opened this issue Feb 15, 2019 · 13 comments

Comments

@dimonomid
Copy link

Hi @hajimehoshi ,

First of all, thanks a lot for your awesome work on go-mp3.

I'm on linux, and while trying it out, I noticed that after the oto.Context has been created, CPU is always at 100% load. E.g. I was following your example https://github.com/hajimehoshi/go-mp3/blob/master/example/main.go , and wrote a function which just plays some mp3 file once:

func playBeep() error {
	f, err := assets.Open("beep.mp3")
	if err != nil {
		return err
	}
	defer f.Close()

	d, err := mp3.NewDecoder(f)
	if err != nil {
		return err
	}
	defer d.Close()

	p, err := oto.NewPlayer(d.SampleRate(), 2, 2, 8192)
	if err != nil {
		return err
	}
	defer p.Close()

	if _, err := io.Copy(p, d); err != nil {
		return err
	}
	return nil
}

And I noticed that after this function was called the first time, my app starts consuming 100% CPU. After some investigation, I found out that oto.NewPlayer() creates a context which is then never closed, and this context is what actually consumes CPU.

However, the code suggests that the context should only be created once: https://github.com/hajimehoshi/oto/blob/master/context.go#L59-L61

	if theContext != nil {
		panic("oto: NewContext can be called only once")
	}

And I noticed that theContext isn't really used anywhere, it only prevents contexts from being created more than once. So I tried to remove that limitation (thus allowing context to be create multiple times), so my playBeep function looks as follows:

func playBeep() error {
	f, err := assets.Open("beep.mp3")
	if err != nil {
		return err
	}
	defer f.Close()

	d, err := mp3.NewDecoder(f)
	if err != nil {
		return err
	}
	defer d.Close()

	c, err := oto.NewContext(d.SampleRate(), 2, 2, 8192)
	if err != nil {
		return err
	}
	defer c.Close()

	p := c.NewPlayer()
	defer p.Close()

	if _, err := io.Copy(p, d); err != nil {
		return err
	}
	return nil
}

And it seems to work fine: plays the sound ever time it's called, and CPU is okay. So, what was the reason for that theContext?

@hajimehoshi
Copy link
Owner

Hi @dimonomid, thank you for reporting!

That sounds a bug in Oto: CPU must not be busy when nothing is played. Let me check. Thank you!

@hajimehoshi
Copy link
Owner

Ah ok, oto.NewPlayer creates a context and a player at the same time, while (*oto.Context).NewPlayer creates a player from a context and this can be called multiple times.

In your case, I think you need only one Context in one process, so would this work?

var theContext *oto.Context

// This is called only once
func initializeContext() error {
        var err error
        theContext, err = oto.NewContext(d.SampleRate(), 2, 2, 8192)
	if err != nil {
		return err
	}
        return nil
}

func playBeep() error {
	f, err := assets.Open("beep.mp3")
	if err != nil {
		return err
	}
	defer f.Close()

	d, err := mp3.NewDecoder(f)
	if err != nil {
		return err
	}
	defer d.Close()

	p := theContext.NewPlayer()
	defer p.Close()

	if _, err := io.Copy(p, d); err != nil {
		return err
	}
	return nil
}

Then you don't have to close (and recreate) the context.

@dimonomid
Copy link
Author

dimonomid commented Feb 15, 2019

Thanks for looking into that!

Not sure what do you mean though; the problem persists: as long as Context is created and not closed, CPU is at 100%. And since in that last example we never close theContext, CPU is always at 100%.

@hajimehoshi
Copy link
Owner

That's odd. I opened a context and wait for 10 seconds, but CPU usage is not changed. Maybe playing something is needed?

@dimonomid
Copy link
Author

dimonomid commented Feb 15, 2019

So it doesn't reproduce on your machine?

I've put together a small repro: https://github.com/dimonomid/mp3test (based on the snippet you posted above)

Just go build && ./mp3test there, and it plays a short sound for me, then sleeps for a minute, and during that sleep CPU is loaded 100%. If I comment the playing part, it's still at 100%. Closing the context does help.

Can you reproduce?

@hajimehoshi
Copy link
Owner

Thanks. I could reproduce this. Let me investigate this. Thank you!

@hajimehoshi
Copy link
Owner

I found the bug in Oto and fixed this: ebitengine/oto@3081a39

Could you test this? Thank you!

@dimonomid
Copy link
Author

It's much better now, but still not ideal: it keeps steadily consuming 5% CPU. If I close the context after playing, it'll be 0% (which is what a sleeping program should do).

dimonomid added a commit to dimonomid/oto that referenced this issue Feb 16, 2019
This is a dirty workaround for an issue of consuming CPU when Context is
idle, see: hajimehoshi/go-mp3#28
@hajimehoshi
Copy link
Owner

it keeps steadily consuming 5% CPU.

My guess is that Oto tries to play 'no sound' when there is no player in the current implementation. Well, suspending the underlying driver during absence of players might work. Let me think.

@ackFacu
Copy link

ackFacu commented Aug 16, 2019

Same problem here. I'm not an expert, but if I find a solution I'm going to share it

@ackFacu
Copy link

ackFacu commented Aug 21, 2019

I made something similar to this and I had no problems. Just a 0,24% cpu usage and 4mb of ram, so idk if this is still a valid issue

func PlayTimeLimit() {
	file, _ := os.Open("notify.mp3")
	defer file.Close()
	decoder, _ := mp3.NewDecoder(file)

	once.Do(func() {
		context, _ = oto.NewContext(decoder.SampleRate(), 2, 2, 8192)
	})

	p := context.NewPlayer()
	defer p.Close()

	if _, err := io.Copy(p, decoder); err != nil {
		return
	}
	return
}

@hajimehoshi
Copy link
Owner

This was already solved, not perfectly though. Theoretically, CPU usage can be 0% when there is no player.

@donething
Copy link

I made something similar to this and I had no problems. Just a 0,24% cpu usage and 4mb of ram, so idk if this is still a valid issue

func PlayTimeLimit() {
	file, _ := os.Open("notify.mp3")
	defer file.Close()
	decoder, _ := mp3.NewDecoder(file)

	once.Do(func() {
		context, _ = oto.NewContext(decoder.SampleRate(), 2, 2, 8192)
	})

	p := context.NewPlayer()
	defer p.Close()

	if _, err := io.Copy(p, decoder); err != nil {
		return
	}
	return
}

Thank you very much,This works.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants