From 319be11f37bdec8aaf47434aa8ffa8fc20e6407f Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Sat, 9 Aug 2025 21:36:04 -0500 Subject: [PATCH 01/12] Add Sound.copy and Sound.__copy__ Co-authored-by: Borishkof --- buildconfig/stubs/pygame/mixer.pyi | 2 ++ docs/reST/ref/mixer.rst | 17 +++++++++++++ src_c/doc/mixer_doc.h | 1 + src_c/mixer.c | 35 ++++++++++++++++++++++++++ test/mixer_test.py | 40 ++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+) diff --git a/buildconfig/stubs/pygame/mixer.pyi b/buildconfig/stubs/pygame/mixer.pyi index 555375c1fd..a6ac4f9476 100644 --- a/buildconfig/stubs/pygame/mixer.pyi +++ b/buildconfig/stubs/pygame/mixer.pyi @@ -72,6 +72,8 @@ class Sound: def get_num_channels(self) -> int: ... def get_length(self) -> float: ... def get_raw(self) -> bytes: ... + def copy(self) -> Sound: ... + def __copy__(self) -> Sound: ... class Channel: def __init__(self, id: int) -> None: ... diff --git a/docs/reST/ref/mixer.rst b/docs/reST/ref/mixer.rst index 69f34e5f10..d5fd932813 100644 --- a/docs/reST/ref/mixer.rst +++ b/docs/reST/ref/mixer.rst @@ -500,6 +500,23 @@ 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. + + Also note that this functions as ``Sound.__copy__``. + + .. versionadded:: 2.5.6 + + .. ## Sound.copy ## + .. ## pygame.mixer.Sound ## .. class:: Channel diff --git a/src_c/doc/mixer_doc.h b/src_c/doc/mixer_doc.h index 92da671ab3..011a0c5868 100644 --- a/src_c/doc/mixer_doc.h +++ b/src_c/doc/mixer_doc.h @@ -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" diff --git a/src_c/mixer.c b/src_c/mixer.c index a31db5454e..bb687fe9f2 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -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 @@ -801,6 +803,37 @@ snd_get_raw(PyObject *self, PyObject *_null) (Py_ssize_t)chunk->alen); } +static PyObject * +snd_copy(PyObject *self, PyObject *_null) +{ + MIXER_INIT_CHECK(); + + pgSoundObject *newSound = + (pgSoundObject *)pgSound_Type.tp_new(&pgSound_Type, NULL, NULL); + + PyObject *kwargs = PyDict_New(); + PyObject *key = PyUnicode_FromString("buffer"); + PyObject *bytes = snd_get_raw(self, NULL); + if (PyDict_SetItem(kwargs, key, bytes) < 0) { + // exception set already + Py_DECREF(key); + Py_DECREF(bytes); + Py_DECREF(kwargs); + return NULL; + } + Py_DECREF(key); + Py_DECREF(bytes); + + if (sound_init((PyObject *)newSound, NULL, kwargs) != 0) { + Py_DECREF(kwargs); + Py_DECREF(newSound); + return RAISE(pgExc_SDLError, "Failed to initialize copied sound"); + } + + Py_DECREF(kwargs); + return (PyObject *)newSound; +} + static PyObject * snd_get_arraystruct(PyObject *self, void *closure) { @@ -858,6 +891,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[] = { diff --git a/test/mixer_test.py b/test/mixer_test.py index 9bc08bf6e9..61b024a79d 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -1330,6 +1330,46 @@ def __init__(self): self.assertRaises(RuntimeError, incorrect.get_volume) + def test_snd_copy(self): + mixer.init() + + filenames = [ + "house_lo.ogg", + "house_lo.wav", + "house_lo.flac", + "house_lo.opus", + "surfonasinewave.xm", + ] + if pygame.mixer.get_sdl_mixer_version() >= (2, 6, 0): + filenames.append("house_lo.mp3") + + for f in filenames: + filename = example_path(os.path.join("data", f)) + try: + sound = mixer.Sound(file=filename) + except pygame.error as e: + 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(0.5) + 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 ##################################### From 48181d3a390e2d8234ad891852dfb5d766e990b2 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Wed, 13 Aug 2025 17:26:10 -0500 Subject: [PATCH 02/12] Fixed refcounting (hopefully) and addressed other review comments from coderabbit --- src_c/mixer.c | 21 +++++++++++++++++---- test/mixer_test.py | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index bb687fe9f2..bf1662655b 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -299,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); @@ -310,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); @@ -806,28 +804,43 @@ snd_get_raw(PyObject *self, PyObject *_null) static PyObject * snd_copy(PyObject *self, PyObject *_null) { + PG_DECLARE_EXCEPTION_SAVER MIXER_INIT_CHECK(); pgSoundObject *newSound = - (pgSoundObject *)pgSound_Type.tp_new(&pgSound_Type, NULL, NULL); + (pgSoundObject *)Py_TYPE(self)->tp_new(&pgSound_Type, NULL, NULL); PyObject *kwargs = PyDict_New(); PyObject *key = PyUnicode_FromString("buffer"); PyObject *bytes = snd_get_raw(self, NULL); + if (bytes == NULL) { + // exception set already by PyBytes_FromStringAndSize + PG_SAVE_EXCEPTION + Py_DECREF(key); + Py_DECREF(kwargs); + PG_UNSAVE_EXCEPTION + return NULL; + } + if (PyDict_SetItem(kwargs, key, bytes) < 0) { // exception set already + PG_SAVE_EXCEPTION Py_DECREF(key); Py_DECREF(bytes); Py_DECREF(kwargs); + PG_UNSAVE_EXCEPTION return NULL; } Py_DECREF(key); Py_DECREF(bytes); if (sound_init((PyObject *)newSound, NULL, kwargs) != 0) { + PG_SAVE_EXCEPTION Py_DECREF(kwargs); Py_DECREF(newSound); - return RAISE(pgExc_SDLError, "Failed to initialize copied sound"); + PG_UNSAVE_EXCEPTION + // Exception set by sound_init + return NULL; } Py_DECREF(kwargs); diff --git a/test/mixer_test.py b/test/mixer_test.py index 61b024a79d..09af7f9983 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -1347,7 +1347,7 @@ def test_snd_copy(self): filename = example_path(os.path.join("data", f)) try: sound = mixer.Sound(file=filename) - except pygame.error as e: + except pygame.error: continue sound_copy = sound.copy() self.assertEqual(sound.get_length(), sound_copy.get_length()) From d5e9157b78b0a6ec0a21775eed6f16001df05c67 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Sat, 9 Aug 2025 22:28:50 -0500 Subject: [PATCH 03/12] Added note about __bytes__ to Sound docs --- docs/reST/ref/mixer.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/reST/ref/mixer.rst b/docs/reST/ref/mixer.rst index d5fd932813..c61f709b1e 100644 --- a/docs/reST/ref/mixer.rst +++ b/docs/reST/ref/mixer.rst @@ -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 From f71f0c3c3767ab5a58e9555817a0ba63647ab5d7 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Fri, 15 Aug 2025 17:15:59 -0500 Subject: [PATCH 04/12] Address comments --- docs/reST/ref/mixer.rst | 3 +++ src_c/mixer.c | 11 ++++++++++- test/mixer_test.py | 41 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/docs/reST/ref/mixer.rst b/docs/reST/ref/mixer.rst index c61f709b1e..82b5161714 100644 --- a/docs/reST/ref/mixer.rst +++ b/docs/reST/ref/mixer.rst @@ -515,6 +515,9 @@ The following file formats are supported 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 diff --git a/src_c/mixer.c b/src_c/mixer.c index bf1662655b..84ddcb2035 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -808,7 +808,7 @@ snd_copy(PyObject *self, PyObject *_null) MIXER_INIT_CHECK(); pgSoundObject *newSound = - (pgSoundObject *)Py_TYPE(self)->tp_new(&pgSound_Type, NULL, NULL); + (pgSoundObject *)Py_TYPE(self)->tp_new(Py_TYPE(self), NULL, NULL); PyObject *kwargs = PyDict_New(); PyObject *key = PyUnicode_FromString("buffer"); @@ -843,6 +843,15 @@ snd_copy(PyObject *self, PyObject *_null) 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(kwargs); return (PyObject *)newSound; } diff --git a/test/mixer_test.py b/test/mixer_test.py index 09af7f9983..03bb24afe6 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -1331,6 +1331,10 @@ 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 = [ @@ -1340,22 +1344,55 @@ def test_snd_copy(self): "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") - for f in filenames: + 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 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.assertTrue(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(0.5) + sound.set_volume(new_vol) self.assertNotEqual(sound.get_volume(), sound_copy.get_volume()) del sound From 9f82126d16ab4d998e7b03d2c8d8ac5fe24e7de3 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Fri, 15 Aug 2025 17:19:54 -0500 Subject: [PATCH 05/12] Halt playback instead of decrefing the sound in mixer_quit --- src_c/mixer.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index 84ddcb2035..5b2cdecca7 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -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); } free(channeldata); From eae838306a6e901b4ee0d4f62faa36b6a8e049be Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Fri, 15 Aug 2025 20:01:53 -0500 Subject: [PATCH 06/12] Review comments from bot, good :bot: --- buildconfig/stubs/pygame/mixer.pyi | 5 ++-- src_c/mixer.c | 38 +++++++++++++++++++++--------- test/mixer_test.py | 34 +++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/buildconfig/stubs/pygame/mixer.pyi b/buildconfig/stubs/pygame/mixer.pyi index a6ac4f9476..309ce5e1a2 100644 --- a/buildconfig/stubs/pygame/mixer.pyi +++ b/buildconfig/stubs/pygame/mixer.pyi @@ -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 ) @@ -72,8 +73,8 @@ class Sound: def get_num_channels(self) -> int: ... def get_length(self) -> float: ... def get_raw(self) -> bytes: ... - def copy(self) -> Sound: ... - def __copy__(self) -> Sound: ... + def copy(self) -> Self: ... + def __copy__(self) -> Self: ... class Channel: def __init__(self, id: int) -> None: ... diff --git a/src_c/mixer.c b/src_c/mixer.c index 5b2cdecca7..a5941a3888 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -816,33 +816,49 @@ snd_copy(PyObject *self, PyObject *_null) pgSoundObject *newSound = (pgSoundObject *)Py_TYPE(self)->tp_new(Py_TYPE(self), NULL, NULL); - PyObject *kwargs = PyDict_New(); - PyObject *key = PyUnicode_FromString("buffer"); + 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(key); - Py_DECREF(kwargs); + Py_DECREF(dict); + Py_DECREF(newSound); PG_UNSAVE_EXCEPTION return NULL; } - if (PyDict_SetItem(kwargs, key, bytes) < 0) { + if (PyDict_SetItemString(dict, "buffer", bytes) < 0) { // exception set already PG_SAVE_EXCEPTION - Py_DECREF(key); Py_DECREF(bytes); - Py_DECREF(kwargs); + Py_DECREF(dict); + Py_DECREF(newSound); PG_UNSAVE_EXCEPTION return NULL; } - Py_DECREF(key); Py_DECREF(bytes); - if (sound_init((PyObject *)newSound, NULL, kwargs) != 0) { + if (sound_init((PyObject *)newSound, NULL, dict) != 0) { PG_SAVE_EXCEPTION - Py_DECREF(kwargs); + Py_DECREF(dict); Py_DECREF(newSound); PG_UNSAVE_EXCEPTION // Exception set by sound_init @@ -858,7 +874,7 @@ snd_copy(PyObject *self, PyObject *_null) Mix_VolumeChunk(dst, vol); } - Py_DECREF(kwargs); + Py_DECREF(dict); return (PyObject *)newSound; } diff --git a/test/mixer_test.py b/test/mixer_test.py index 03bb24afe6..e1b17cadc0 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -1,3 +1,4 @@ +import copy import os import pathlib import platform @@ -1348,6 +1349,8 @@ def __init__(self, *args, **kwargs): 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)) @@ -1377,6 +1380,35 @@ def __init__(self, *args, **kwargs): 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)) @@ -1386,7 +1418,7 @@ def __init__(self, *args, **kwargs): except pygame.error: continue sound_copy = sound.copy() - self.assertTrue(sound_copy, SubSound) + 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()) From 4e11fadb35dde149afe2bd36719f279eceb601d0 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Fri, 15 Aug 2025 20:26:51 -0500 Subject: [PATCH 07/12] More refcount edits --- src_c/mixer.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index a5941a3888..7572e2e25c 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -299,6 +299,7 @@ endsound_callback(int channel) PyGILState_STATE gstate = PyGILState_Ensure(); int channelnum; Mix_Chunk *sound = pgSound_AsChunk(channeldata[channel].queue); + Py_DECREF(channeldata[channel].sound); channeldata[channel].sound = channeldata[channel].queue; channeldata[channel].queue = NULL; PyGILState_Release(gstate); @@ -309,6 +310,7 @@ endsound_callback(int channel) } else { PyGILState_STATE gstate = PyGILState_Ensure(); + Py_DECREF(channeldata[channel].sound); channeldata[channel].sound = NULL; PyGILState_Release(gstate); Mix_GroupChannel(channel, -1); @@ -676,7 +678,11 @@ pgSound_Play(PyObject *self, PyObject *args, PyObject *kwargs) Py_RETURN_NONE; } - Py_XDECREF(channeldata[channelnum].sound); + if (channeldata[channelnum].sound) { + Mix_HaltGroup( + (int)(intptr_t)pgSound_AsChunk(channeldata[channelnum].sound)); + } + Py_XDECREF(channeldata[channelnum].queue); channeldata[channelnum].queue = NULL; channeldata[channelnum].sound = self; From 0a657669a8b261d27055c3070895426c04d46b64 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Fri, 15 Aug 2025 21:17:13 -0500 Subject: [PATCH 08/12] Put decref back --- src_c/mixer.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index 7572e2e25c..4ac5981e45 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -519,14 +519,14 @@ mixer_quit(PyObject *self, PyObject *_null) if (channeldata) { for (i = 0; i < numchanneldata; ++i) { - if (channeldata[i].queue) { + if (channeldata[i].sound) { Mix_HaltGroup( (int)(intptr_t)pgSound_AsChunk(channeldata[i].sound)); } - else { - Mix_HaltGroup(-1); + if (channeldata[i].queue) { + Mix_HaltGroup( + (int)(intptr_t)pgSound_AsChunk(channeldata[i].queue)); } - Py_XDECREF(channeldata[i].queue); } free(channeldata); channeldata = NULL; @@ -678,11 +678,7 @@ pgSound_Play(PyObject *self, PyObject *args, PyObject *kwargs) Py_RETURN_NONE; } - if (channeldata[channelnum].sound) { - Mix_HaltGroup( - (int)(intptr_t)pgSound_AsChunk(channeldata[channelnum].sound)); - } - + Py_XDECREF(channeldata[channelnum].sound); Py_XDECREF(channeldata[channelnum].queue); channeldata[channelnum].queue = NULL; channeldata[channelnum].sound = self; From 669f06eebc5fc4bdd99d85ce3a80368d65a6d9f4 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Fri, 15 Aug 2025 21:23:21 -0500 Subject: [PATCH 09/12] Use XDECREF where it is potentially unsafe to use normal DECREF --- src_c/mixer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index 4ac5981e45..5454f69377 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -299,7 +299,7 @@ endsound_callback(int channel) PyGILState_STATE gstate = PyGILState_Ensure(); int channelnum; Mix_Chunk *sound = pgSound_AsChunk(channeldata[channel].queue); - Py_DECREF(channeldata[channel].sound); + Py_XDECREF(channeldata[channel].sound); channeldata[channel].sound = channeldata[channel].queue; channeldata[channel].queue = NULL; PyGILState_Release(gstate); @@ -310,7 +310,7 @@ endsound_callback(int channel) } else { PyGILState_STATE gstate = PyGILState_Ensure(); - Py_DECREF(channeldata[channel].sound); + Py_XDECREF(channeldata[channel].sound); channeldata[channel].sound = NULL; PyGILState_Release(gstate); Mix_GroupChannel(channel, -1); From 24348bc5f61dcfd68a1bae60d65f7ae44cab519c Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Sun, 17 Aug 2025 08:47:45 -0500 Subject: [PATCH 10/12] Review comments --- src_c/mixer.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index 5454f69377..d6a786c893 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -812,7 +812,6 @@ snd_get_raw(PyObject *self, PyObject *_null) static PyObject * snd_copy(PyObject *self, PyObject *_null) { - PG_DECLARE_EXCEPTION_SAVER MIXER_INIT_CHECK(); pgSoundObject *newSound = @@ -828,11 +827,6 @@ snd_copy(PyObject *self, PyObject *_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; } @@ -840,29 +834,23 @@ snd_copy(PyObject *self, PyObject *_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; } From 58c24a6a70371f6909b56f9c9dea042254423857 Mon Sep 17 00:00:00 2001 From: Dan Lawrence Date: Mon, 25 Aug 2025 11:20:16 +0100 Subject: [PATCH 11/12] add Mix_ChannelFinished from review --- src_c/mixer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src_c/mixer.c b/src_c/mixer.c index d6a786c893..542206a837 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -516,6 +516,7 @@ mixer_quit(PyObject *self, PyObject *_null) Py_BEGIN_ALLOW_THREADS; Mix_HaltMusic(); Py_END_ALLOW_THREADS; +Mix_ChannelFinished(NULL); if (channeldata) { for (i = 0; i < numchanneldata; ++i) { From e6704eaa6b42cd88ccd38cda80ea31ab728c0d91 Mon Sep 17 00:00:00 2001 From: Dan Lawrence Date: Mon, 25 Aug 2025 11:21:36 +0100 Subject: [PATCH 12/12] fix spacing --- src_c/mixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index 542206a837..27925128ca 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -516,7 +516,7 @@ mixer_quit(PyObject *self, PyObject *_null) Py_BEGIN_ALLOW_THREADS; Mix_HaltMusic(); Py_END_ALLOW_THREADS; -Mix_ChannelFinished(NULL); + Mix_ChannelFinished(NULL); if (channeldata) { for (i = 0; i < numchanneldata; ++i) {