From 532482aebbf0c360252947296810a56bdfcba644 Mon Sep 17 00:00:00 2001 From: bilhox <69472620+bilhox@users.noreply.github.com> Date: Sat, 5 Oct 2024 11:50:07 +0200 Subject: [PATCH 1/9] sound copy part 1 --- src_c/mixer.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src_c/mixer.c b/src_c/mixer.c index 0d3fa8f393..a486c40575 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -831,6 +831,31 @@ snd_get_samples_address(PyObject *self, PyObject *closure) #endif } +static PyObject * +snd_copy(pgSoundObject *self, PyObject *_null) +{ + Mix_Chunk *chunk = pgSound_AsChunk(self); + pgSoundObject *new_sound; + Mix_Chunk *new_chunk; + + CHECK_CHUNK_VALID(chunk, -1); + + new_sound = + (pgSoundObject *)pgSound_Type.tp_new(Py_TYPE(self), NULL, NULL); + + if (chunk->allocated) { + new_chunk = Mix_QuickLoad_RAW(chunk->abuf, chunk->alen); + if (!new_chunk) + RAISE(pgExc_BufferError, + "there is a problem I couldn't describe ;-;"); + + new_chunk->volume = chunk->volume; + new_sound->chunk = new_chunk; + } + + return (PyObject *)new_sound; +} + PyMethodDef sound_methods[] = { {"play", (PyCFunction)pgSound_Play, METH_VARARGS | METH_KEYWORDS, DOC_MIXER_SOUND_PLAY}, @@ -842,6 +867,7 @@ 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, ""}, {NULL, NULL, 0, NULL}}; static PyGetSetDef sound_getset[] = { From 40bacbb471d14664ef1c8cc3fcd7886042ea7ffc Mon Sep 17 00:00:00 2001 From: bilhox <69472620+bilhox@users.noreply.github.com> Date: Wed, 23 Oct 2024 12:04:30 +0200 Subject: [PATCH 2/9] error message --- src_c/mixer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index a486c40575..42701236f4 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -846,8 +846,7 @@ snd_copy(pgSoundObject *self, PyObject *_null) if (chunk->allocated) { new_chunk = Mix_QuickLoad_RAW(chunk->abuf, chunk->alen); if (!new_chunk) - RAISE(pgExc_BufferError, - "there is a problem I couldn't describe ;-;"); + RAISE(pgExc_BufferError, "there is a problem I can't describe :)"); new_chunk->volume = chunk->volume; new_sound->chunk = new_chunk; From 1fd41e2a17e5ec6376b42b1634d48028a62ac9c2 Mon Sep 17 00:00:00 2001 From: Borishkof Date: Sun, 24 Nov 2024 16:21:18 +0100 Subject: [PATCH 3/9] snd_cpoy tests --- test/mixer_test.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/mixer_test.py b/test/mixer_test.py index 9b3551b834..0350d60d7a 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -710,6 +710,28 @@ def test_get_sdl_mixer_version__linked_equals_compiled(self): compiled_version = pygame.mixer.get_sdl_mixer_version(linked=False) self.assertTupleEqual(linked_version, compiled_version) + + def test_snd_copy(self): + mixer.init() + filename = example_path(os.path.join("data", "house_lo.wav")) + sound = mixer.Sound(file=filename) + 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()) + + # Destroy the original sound + del sound + + # Test on the copy + channel = sound_copy.play() + self.assertTrue(channel.get_busy()) + sound_copy.stop() + self.assertFalse(channel.get_busy()) + + sound_copy.play() + self.assertEqual(sound_copy.get_num_channels(), 1) ############################## CHANNEL CLASS TESTS ############################# From aec7da70ae39fd57ef57ca54f8770e5d1825f7c9 Mon Sep 17 00:00:00 2001 From: Borishkof Date: Sun, 24 Nov 2024 18:20:53 +0100 Subject: [PATCH 4/9] sound copy first fix and better tests --- src_c/mixer.c | 52 +++++++++++++++++++++++++++++++++++++++++----- test/mixer_test.py | 20 +++++++++++++----- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src_c/mixer.c b/src_c/mixer.c index 42701236f4..27e70a5d1b 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -838,19 +838,61 @@ snd_copy(pgSoundObject *self, PyObject *_null) pgSoundObject *new_sound; Mix_Chunk *new_chunk; - CHECK_CHUNK_VALID(chunk, -1); + // Validate the input chunk + CHECK_CHUNK_VALID(chunk, NULL); + // Create a new sound object new_sound = (pgSoundObject *)pgSound_Type.tp_new(Py_TYPE(self), NULL, NULL); + if (!new_sound) { + PyErr_SetString(PyExc_RuntimeError, + "Failed to allocate memory for new sound object"); + return NULL; + } + // Handle chunk allocation type if (chunk->allocated) { - new_chunk = Mix_QuickLoad_RAW(chunk->abuf, chunk->alen); - if (!new_chunk) - RAISE(pgExc_BufferError, "there is a problem I can't describe :)"); - + // Create a deep copy of the audio buffer for allocated chunks + Uint8 *buffer_copy = (Uint8 *)malloc(chunk->alen); + if (!buffer_copy) { + Py_DECREF(new_sound); + PyErr_SetString(PyExc_MemoryError, + "Failed to allocate memory for sound buffer"); + return NULL; + } + memcpy(buffer_copy, chunk->abuf, chunk->alen); + + // Create a new Mix_Chunk + new_chunk = Mix_QuickLoad_RAW(buffer_copy, chunk->alen); + if (!new_chunk) { + free(buffer_copy); + Py_DECREF(new_sound); + PyErr_SetString(PyExc_RuntimeError, + "Failed to create new sound chunk"); + return NULL; + } new_chunk->volume = chunk->volume; new_sound->chunk = new_chunk; } + else { + // For non-allocated chunks (e.g., formats like .xm), create a full + // copy + new_chunk = (Mix_Chunk *)malloc(sizeof(Mix_Chunk)); + if (!new_chunk) { + Py_DECREF(new_sound); + PyErr_SetString(PyExc_MemoryError, + "Failed to allocate memory for sound chunk"); + return NULL; + } + *new_chunk = *chunk; // Copy the entire structure + + // For safety, ensure the copied chunk doesn't share pointers + new_chunk->abuf = + NULL; // Prevent double-free if original gets deallocated + new_chunk->allocated = 0; + + new_sound->chunk = new_chunk; + } return (PyObject *)new_sound; } diff --git a/test/mixer_test.py b/test/mixer_test.py index 0350d60d7a..39515c2986 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -710,10 +710,21 @@ def test_get_sdl_mixer_version__linked_equals_compiled(self): compiled_version = pygame.mixer.get_sdl_mixer_version(linked=False) self.assertTupleEqual(linked_version, compiled_version) - - def test_snd_copy(self): - mixer.init() - filename = example_path(os.path.join("data", "house_lo.wav")) + + def test_snd_copy(self): + mixer.init() + + filenames = [ + "house_lo.mp3", + "house_lo.ogg", + "house_lo.wav", + "house_lo.flac", + # "house_lo.opus", unsupported + # "surfonasinewave.xm" unsupported + ] + + for f in filenames: + filename = example_path(os.path.join("data", f)) sound = mixer.Sound(file=filename) sound_copy = sound.copy() self.assertEqual(sound.get_length(), sound_copy.get_length()) @@ -721,7 +732,6 @@ def test_snd_copy(self): self.assertEqual(sound.get_volume(), sound_copy.get_volume()) self.assertEqual(sound.get_raw(), sound_copy.get_raw()) - # Destroy the original sound del sound # Test on the copy From 3be9a1cb56743351b29765db23fb04a4ca9a9848 Mon Sep 17 00:00:00 2001 From: Borishkof Date: Wed, 27 Nov 2024 14:35:37 +0100 Subject: [PATCH 5/9] Documentation for pygame.Sound.copy. Parameter type changing from pgSoundObject to PyObject. Co-authored-by: Thomasgrgi Co-authored-by: Bilhox Co-authored-by: AntoineM " --- docs/reST/ref/mixer.rst | 11 +++++++++++ src_c/doc/mixer_doc.h | 1 + src_c/mixer.c | 4 ++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/reST/ref/mixer.rst b/docs/reST/ref/mixer.rst index 5a441c7f81..6fc1e6a530 100644 --- a/docs/reST/ref/mixer.rst +++ b/docs/reST/ref/mixer.rst @@ -479,6 +479,17 @@ The following file formats are supported .. ## Sound.get_raw ## + .. method:: copy + + | :sl:`return a new Sound object that is a deep copy of this one` + | :sg:`copy() -> Sound` + + Return a new Sound object that is a deep copy of this one. The new Sound will + be playable just like the original. If the copy fails, the method returns ``NULL``. + + .. ## 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..e3ed8f9144 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\nreturn a new Sound object that is a deep copy of this one" #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 27e70a5d1b..fa1113cff5 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -832,7 +832,7 @@ snd_get_samples_address(PyObject *self, PyObject *closure) } static PyObject * -snd_copy(pgSoundObject *self, PyObject *_null) +snd_copy(PyObject *self, PyObject *_null) { Mix_Chunk *chunk = pgSound_AsChunk(self); pgSoundObject *new_sound; @@ -908,7 +908,7 @@ 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, ""}, + {"copy", snd_copy, METH_NOARGS, DOC_MIXER_SOUND_COPY}, {NULL, NULL, 0, NULL}}; static PyGetSetDef sound_getset[] = { From 726244d8d2b7d0226cb07a82f727bd4c8ca6425d Mon Sep 17 00:00:00 2001 From: Borishkof Date: Wed, 27 Nov 2024 15:21:15 +0100 Subject: [PATCH 6/9] Better documentation for pygame.sound.copy --- docs/reST/ref/mixer.rst | 3 ++- src_c/mixer.c | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/reST/ref/mixer.rst b/docs/reST/ref/mixer.rst index 6fc1e6a530..c5041c8eb3 100644 --- a/docs/reST/ref/mixer.rst +++ b/docs/reST/ref/mixer.rst @@ -485,7 +485,8 @@ The following file formats are supported | :sg:`copy() -> Sound` Return a new Sound object that is a deep copy of this one. The new Sound will - be playable just like the original. If the copy fails, the method returns ``NULL``. + be playable just like the original. If the copy fails, a ``MemoryError`` or a + :meth:`pygame.error` exception will be raised. .. ## Sound.copy ## diff --git a/src_c/mixer.c b/src_c/mixer.c index fa1113cff5..72d5ba8d7f 100644 --- a/src_c/mixer.c +++ b/src_c/mixer.c @@ -845,7 +845,7 @@ snd_copy(PyObject *self, PyObject *_null) new_sound = (pgSoundObject *)pgSound_Type.tp_new(Py_TYPE(self), NULL, NULL); if (!new_sound) { - PyErr_SetString(PyExc_RuntimeError, + PyErr_SetString(PyExc_MemoryError, "Failed to allocate memory for new sound object"); return NULL; } @@ -867,7 +867,7 @@ snd_copy(PyObject *self, PyObject *_null) if (!new_chunk) { free(buffer_copy); Py_DECREF(new_sound); - PyErr_SetString(PyExc_RuntimeError, + PyErr_SetString(pgExc_SDLError, "Failed to create new sound chunk"); return NULL; } @@ -909,6 +909,7 @@ PyMethodDef sound_methods[] = { {"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[] = { From 4bfd1e85062afbd8c107068ac3dd87acc5ebc24c Mon Sep 17 00:00:00 2001 From: Borishkof Date: Thu, 28 Nov 2024 15:55:36 +0100 Subject: [PATCH 7/9] stubs addition and fixing mp3 test --- buildconfig/stubs/pygame/mixer.pyi | 2 ++ test/mixer_test.py | 11 +++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/buildconfig/stubs/pygame/mixer.pyi b/buildconfig/stubs/pygame/mixer.pyi index 84646b419b..fc04a0ef02 100644 --- a/buildconfig/stubs/pygame/mixer.pyi +++ b/buildconfig/stubs/pygame/mixer.pyi @@ -70,6 +70,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: diff --git a/test/mixer_test.py b/test/mixer_test.py index 39515c2986..4d36dafb75 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -714,14 +714,9 @@ def test_get_sdl_mixer_version__linked_equals_compiled(self): def test_snd_copy(self): mixer.init() - filenames = [ - "house_lo.mp3", - "house_lo.ogg", - "house_lo.wav", - "house_lo.flac", - # "house_lo.opus", unsupported - # "surfonasinewave.xm" unsupported - ] + filenames = ["house_lo.ogg", "house_lo.wav", "house_lo.flac"] + 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)) From 446c2f1608394b41489380340ebe93f3594456e3 Mon Sep 17 00:00:00 2001 From: Borishkof Date: Fri, 29 Nov 2024 20:34:01 +0100 Subject: [PATCH 8/9] "Add xm and opus to tested formats Co-authored-by: Bilhox " --- test/mixer_test.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/mixer_test.py b/test/mixer_test.py index 4d36dafb75..297454935e 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -714,7 +714,13 @@ def test_get_sdl_mixer_version__linked_equals_compiled(self): def test_snd_copy(self): mixer.init() - filenames = ["house_lo.ogg", "house_lo.wav", "house_lo.flac"] + 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") @@ -727,14 +733,18 @@ def test_snd_copy(self): 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 + # 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) From cc67e0db4151bf9f1633f16bfbc48353f51a4199 Mon Sep 17 00:00:00 2001 From: Borishkof Date: Sat, 30 Nov 2024 11:05:18 +0100 Subject: [PATCH 9/9] safe loading files format --- test/mixer_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/mixer_test.py b/test/mixer_test.py index 297454935e..7c1c4794b6 100644 --- a/test/mixer_test.py +++ b/test/mixer_test.py @@ -726,7 +726,10 @@ def test_snd_copy(self): for f in filenames: filename = example_path(os.path.join("data", f)) - sound = mixer.Sound(file=filename) + 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())