Skip to content

Add Sound.copy and Sound.__copy__ #3556

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions buildconfig/stubs/pygame/mixer.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ from pygame.event import Event
from pygame.typing import FileLike
from typing_extensions import (
Buffer, # collections.abc 3.12
Self,
deprecated, # added in 3.13
)

Expand Down Expand Up @@ -72,6 +73,8 @@ class Sound:
def get_num_channels(self) -> int: ...
def get_length(self) -> float: ...
def get_raw(self) -> bytes: ...
def copy(self) -> Self: ...
def __copy__(self) -> Self: ...

class Channel:
def __init__(self, id: int) -> None: ...
Expand Down
24 changes: 24 additions & 0 deletions docs/reST/ref/mixer.rst
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,10 @@ The following file formats are supported
an exception when different. Also, source samples are truncated to fit the
audio sample size. This will not change.

.. note:: ``bytes(Sound)`` and ``bytearray(Sound)`` make use of the buffer
interface, which is implemented internally by ``pygame.mixer.Sound``.
Because of this, there is no need to directly implement ``__bytes__``.

.. versionaddedold:: 1.8 ``pygame.mixer.Sound(buffer)``
.. versionaddedold:: 1.9.2
:class:`pygame.mixer.Sound` keyword arguments and array interface support
Expand Down Expand Up @@ -500,6 +504,26 @@ The following file formats are supported

.. ## Sound.get_raw ##

.. method:: copy

| :sl:`return a new Sound object that is a deep copy of this Sound`
| :sg:`copy() -> Sound`
| :sg:`copy.copy(original_sound) -> Sound`

Return a new Sound object that is a deep copy of this Sound. The new Sound
will be just as if you loaded it from the same file on disk as you did the
original Sound. If the copy fails, a ``TypeError`` or :meth:`pygame.error`
exception will be raised.

If copying a subclass of ``mixer.Sound``, an instance of the same subclass
will be returned.

Also note that this functions as ``Sound.__copy__``.

.. versionadded:: 2.5.6

.. ## Sound.copy ##

.. ## pygame.mixer.Sound ##

.. class:: Channel
Expand Down
1 change: 1 addition & 0 deletions src_c/doc/mixer_doc.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define DOC_MIXER_SOUND_GETNUMCHANNELS "get_num_channels() -> count\ncount how many times this Sound is playing"
#define DOC_MIXER_SOUND_GETLENGTH "get_length() -> seconds\nget the length of the Sound"
#define DOC_MIXER_SOUND_GETRAW "get_raw() -> bytes\nreturn a bytestring copy of the Sound samples."
#define DOC_MIXER_SOUND_COPY "copy() -> Sound\ncopy.copy(original_sound) -> Sound\nreturn a new Sound object that is a deep copy of this Sound"
#define DOC_MIXER_CHANNEL "Channel(id) -> Channel\nCreate a Channel object for controlling playback"
#define DOC_MIXER_CHANNEL_ID "id -> int\nget the channel id for the Channel object"
#define DOC_MIXER_CHANNEL_PLAY "play(Sound, loops=0, maxtime=0, fade_ms=0) -> None\nplay a Sound on a specific Channel"
Expand Down
85 changes: 82 additions & 3 deletions src_c/mixer.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ pgChannel_New(int);
#define pgChannel_Check(x) \
(PyObject_IsInstance(x, (PyObject *)&pgChannel_Type))

static PyObject *
snd_get_arraystruct(PyObject *self, void *closure);
static int
snd_getbuffer(PyObject *, Py_buffer *, int);
static void
Expand Down Expand Up @@ -297,7 +299,6 @@ endsound_callback(int channel)
PyGILState_STATE gstate = PyGILState_Ensure();
int channelnum;
Mix_Chunk *sound = pgSound_AsChunk(channeldata[channel].queue);
Py_XDECREF(channeldata[channel].sound);
channeldata[channel].sound = channeldata[channel].queue;
channeldata[channel].queue = NULL;
PyGILState_Release(gstate);
Expand All @@ -308,7 +309,6 @@ endsound_callback(int channel)
}
else {
PyGILState_STATE gstate = PyGILState_Ensure();
Py_XDECREF(channeldata[channel].sound);
channeldata[channel].sound = NULL;
PyGILState_Release(gstate);
Mix_GroupChannel(channel, -1);
Expand Down Expand Up @@ -517,7 +517,13 @@ mixer_quit(PyObject *self, PyObject *_null)

if (channeldata) {
for (i = 0; i < numchanneldata; ++i) {
Py_XDECREF(channeldata[i].sound);
if (channeldata[i].queue) {
Mix_HaltGroup(
(int)(intptr_t)pgSound_AsChunk(channeldata[i].sound));
}
else {
Mix_HaltGroup(-1);
}
Py_XDECREF(channeldata[i].queue);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Leak: channeldata[i].sound is never decref’d in mixer_quit

Inside the loop, channeldata[i].sound holds a reference to a Python object. The code decrefs queue but never decrefs sound, leaking a ref per channel. Add Py_XDECREF(channeldata[i].sound); before freeing channeldata.

Apply this diff:

                 if (channeldata[i].queue) {
                     Mix_HaltGroup(
                         (int)(intptr_t)pgSound_AsChunk(channeldata[i].sound));
                 }
                 else {
                     Mix_HaltGroup(-1);
                 }
+                Py_XDECREF(channeldata[i].sound);
                 Py_XDECREF(channeldata[i].queue);

Nit (optional): Calling Mix_HaltGroup(-1) inside the loop repeats the same work N times; you can do it once outside the loop for the non-queue case.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (channeldata) {
for (i = 0; i < numchanneldata; ++i) {
Py_XDECREF(channeldata[i].sound);
if (channeldata[i].queue) {
Mix_HaltGroup(
(int)(intptr_t)pgSound_AsChunk(channeldata[i].sound));
}
else {
Mix_HaltGroup(-1);
}
Py_XDECREF(channeldata[i].queue);
if (channeldata) {
for (i = 0; i < numchanneldata; ++i) {
if (channeldata[i].queue) {
Mix_HaltGroup(
(int)(intptr_t)pgSound_AsChunk(channeldata[i].sound));
}
else {
Mix_HaltGroup(-1);
}
Py_XDECREF(channeldata[i].sound);
Py_XDECREF(channeldata[i].queue);
🤖 Prompt for AI Agents
In src_c/mixer.c around lines 518 to 527, the loop in mixer_quit decrefs
channeldata[i].queue but never decrefs channeldata[i].sound, leaking one Python
reference per channel; add a Py_XDECREF(channeldata[i].sound) for each iteration
before you free channeldata. Also, to avoid repeating work, call
Mix_HaltGroup(-1) once outside the loop for the non-queue case instead of
calling it on every iteration. Ensure the added Py_XDECREF is used (not
Py_DECREF) to safely handle NULL values.

}
free(channeldata);
Expand Down Expand Up @@ -801,6 +807,77 @@ snd_get_raw(PyObject *self, PyObject *_null)
(Py_ssize_t)chunk->alen);
}

static PyObject *
snd_copy(PyObject *self, PyObject *_null)
{
PG_DECLARE_EXCEPTION_SAVER
MIXER_INIT_CHECK();

pgSoundObject *newSound =
(pgSoundObject *)Py_TYPE(self)->tp_new(Py_TYPE(self), NULL, NULL);

if (!newSound) {
if (!PyErr_Occurred()) {
PyErr_SetString(PyExc_MemoryError,
"Failed to create new Sound object for copy");
}
return NULL;
}

PyObject *dict = PyDict_New();
if (!dict) {
if (!PyErr_Occurred()) {
PyErr_SetString(PyExc_MemoryError,
"Failed to create internal dictionary to create "
"copy of Sound");
}
Py_DECREF(newSound);
return NULL;
}

PyObject *bytes = snd_get_raw(self, NULL);
if (bytes == NULL) {
// exception set already by PyBytes_FromStringAndSize
PG_SAVE_EXCEPTION
Py_DECREF(dict);
Py_DECREF(newSound);
PG_UNSAVE_EXCEPTION
return NULL;
}

if (PyDict_SetItemString(dict, "buffer", bytes) < 0) {
// exception set already
PG_SAVE_EXCEPTION
Py_DECREF(bytes);
Py_DECREF(dict);
Py_DECREF(newSound);
PG_UNSAVE_EXCEPTION
return NULL;
}
Py_DECREF(bytes);

if (sound_init((PyObject *)newSound, NULL, dict) != 0) {
PG_SAVE_EXCEPTION
Py_DECREF(dict);
Py_DECREF(newSound);
PG_UNSAVE_EXCEPTION
// Exception set by sound_init
return NULL;
}

// Preserve original volume on the new chunk
Mix_Chunk *orig = pgSound_AsChunk(self);
Mix_Chunk *dst = pgSound_AsChunk((PyObject *)newSound);

if (orig && dst) {
int vol = Mix_VolumeChunk(orig, -1);
Mix_VolumeChunk(dst, vol);
}

Py_DECREF(dict);
return (PyObject *)newSound;
}

static PyObject *
snd_get_arraystruct(PyObject *self, void *closure)
{
Expand Down Expand Up @@ -858,6 +935,8 @@ PyMethodDef sound_methods[] = {
{"get_volume", snd_get_volume, METH_NOARGS, DOC_MIXER_SOUND_GETVOLUME},
{"get_length", snd_get_length, METH_NOARGS, DOC_MIXER_SOUND_GETLENGTH},
{"get_raw", snd_get_raw, METH_NOARGS, DOC_MIXER_SOUND_GETRAW},
{"copy", snd_copy, METH_NOARGS, DOC_MIXER_SOUND_COPY},
{"__copy__", snd_copy, METH_NOARGS, DOC_MIXER_SOUND_COPY},
{NULL, NULL, 0, NULL}};

static PyGetSetDef sound_getset[] = {
Expand Down
109 changes: 109 additions & 0 deletions test/mixer_test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import os
import pathlib
import platform
Expand Down Expand Up @@ -1330,6 +1331,114 @@ def __init__(self):

self.assertRaises(RuntimeError, incorrect.get_volume)

def test_snd_copy(self):
class SubSound(mixer.Sound):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

mixer.init()

filenames = [
"house_lo.ogg",
"house_lo.wav",
"house_lo.flac",
"house_lo.opus",
"surfonasinewave.xm",
]
old_volumes = [0.1, 0.2, 0.5, 0.7, 1.0]
new_volumes = [0.2, 0.3, 0.7, 1.0, 0.1]
if pygame.mixer.get_sdl_mixer_version() >= (2, 6, 0):
filenames.append("house_lo.mp3")
old_volumes.append(0.9)
new_volumes.append(0.5)

for f, old_vol, new_vol in zip(filenames, old_volumes, new_volumes):
filename = example_path(os.path.join("data", f))
try:
sound = mixer.Sound(file=filename)
sound.set_volume(old_vol)
except pygame.error:
continue
sound_copy = sound.copy()
self.assertEqual(sound.get_length(), sound_copy.get_length())
self.assertEqual(sound.get_num_channels(), sound_copy.get_num_channels())
self.assertEqual(sound.get_volume(), sound_copy.get_volume())
self.assertEqual(sound.get_raw(), sound_copy.get_raw())

sound.set_volume(new_vol)
self.assertNotEqual(sound.get_volume(), sound_copy.get_volume())

del sound

# Test on the copy for playable sounds
channel = sound_copy.play()
if channel is None:
continue
self.assertTrue(channel.get_busy())
sound_copy.stop()
self.assertFalse(channel.get_busy())
sound_copy.play()
self.assertEqual(sound_copy.get_num_channels(), 1)

# Test __copy__
for f, old_vol, new_vol in zip(filenames, old_volumes, new_volumes):
filename = example_path(os.path.join("data", f))
try:
sound = mixer.Sound(file=filename)
sound.set_volume(old_vol)
except pygame.error:
continue
sound_copy = copy.copy(sound)
self.assertEqual(sound.get_length(), sound_copy.get_length())
self.assertEqual(sound.get_num_channels(), sound_copy.get_num_channels())
self.assertEqual(sound.get_volume(), sound_copy.get_volume())
self.assertEqual(sound.get_raw(), sound_copy.get_raw())

sound.set_volume(new_vol)
self.assertNotEqual(sound.get_volume(), sound_copy.get_volume())

del sound

# Test on the copy for playable sounds
channel = sound_copy.play()
if channel is None:
continue
self.assertTrue(channel.get_busy())
sound_copy.stop()
self.assertFalse(channel.get_busy())
sound_copy.play()
self.assertEqual(sound_copy.get_num_channels(), 1)

# Test copying a subclass of Sound
for f, old_vol, new_vol in zip(filenames, old_volumes, new_volumes):
filename = example_path(os.path.join("data", f))
try:
sound = SubSound(file=filename)
sound.set_volume(old_vol)
except pygame.error:
continue
sound_copy = sound.copy()
self.assertIsInstance(sound_copy, SubSound)
self.assertEqual(sound.get_length(), sound_copy.get_length())
self.assertEqual(sound.get_num_channels(), sound_copy.get_num_channels())
self.assertEqual(sound.get_volume(), sound_copy.get_volume())
self.assertEqual(sound.get_raw(), sound_copy.get_raw())

sound.set_volume(new_vol)
self.assertNotEqual(sound.get_volume(), sound_copy.get_volume())

del sound

# Test on the copy for playable sounds
channel = sound_copy.play()
if channel is None:
continue
self.assertTrue(channel.get_busy())
sound_copy.stop()
self.assertFalse(channel.get_busy())
sound_copy.play()
self.assertEqual(sound_copy.get_num_channels(), 1)


##################################### MAIN #####################################

Expand Down
Loading