pygame.music.Music implementation

[bilhox]

Hello everyone,

As discussed on discord and on #3058 , I want to introduce a new pygame object that will help users to manipulate their musics in a better way, and also in a more pythonic way.
The goal of this issue to collect feedbacks on how it should be implemented before opening a PR for its implementation.

Implementation

pygame.Music is an alias of pygame.music.Music.
pygame.music should be an alias for pygame.mixer_music.

class Music:
    def __init__(self, path):

        self.metadata : dict
        # or
        self.title : str
        self.album : str
        self.artist : str
        self.copyright : str

        self.position : float # Settable / Gettable (set/get from where it should/is play(ing))
        self.duration : float # Read-only (returned time is in seconds just like SDL Mixer 2.6.0 functions use)
        self.playing : bool # Read-only (Could be pygame.mixer.music.is_playing(Music) ?)
        self.fade_in : float # Gettable / Settable (the value is in seconds)
        self.fade_out : float # Gettable / Settable (the value is in seconds)
        self.paused : bool # Gettable / Settable
        self.ended : bool # Read-only (True only if the position >= duration or music stopped)

        # If one day it's doable to play multiple musics at the same time, we could have
        self.volume : float # Gettable / Settable
    
    # For each method below, their equivalent in pygame.mixer.music has the same behaviour

    def play() -> None: ... 
    def rewind() -> None: ...
    def stop() -> None: ...

# New module functions
pygame.mixer_music.get_music() -> pygame.Music | None # Returns the music loaded

path is not necessary a str but support all types that pygame supported before.

I don't like how pygame.mixer.music.set|get_endevent works, it's not something the user have to setup. This is why I propose a better way IMO, when Music.stop() or pygame.mixer.music.stop() is called, or when the music ended peacefully, a pygame.MUSICENDED is sent to the event queue. This event would have a single attribute, music which is the music object that ended.

I highly support the metadata attributes than an actual metadata dict attribute, imo it doesn't look cool to write. Btw, what about an attribute for the music format type (ogg, wav, mp3 ...) ?

Why the need of this feature ?

For many reasons :

• It helps pygame users to correctly configure their musics in a more pythonic way.
• It allows to queue a music with specific fade_in and fade_out, which wasn't possible with the old implementation.
• If in a future, one day, we can play multiple musics at the same time, the system would be more friendly.
• Fixes how pygame.mixer.music.set_pos worked on the old implementation, which was definitly wrong. Also bring a homogeneous measure system (before get_pos was returning the value in ms, and set_pos needed a value in seconds).
• So we can forget the cursed code we can see in music.c

Updates

• Music.is_playing got renamed to Music.playing
• MUSICENDED event could have a stopped attribute so the user can now if the music got brutally stopped by a hater or not.

[damusss]

Yes.
Please enlighten my feedback with yours, oh majestic comment reader.

Most important detail

I think this object should be contained in the pygame.music module. This should be a "modern" api, disconnected from the old api. having it in a new module means:

• it can easily be "marked" as experimental, just like geometry. you can't easily "unimport" a class that is inside a "stable" module like mixer_music
• This is because this object completely removes the need for mixer_music. so it shouldn't live there. people should gradually switch to this module and forget about the old one. this is like draw compared to gfxdraw.
• What kind of a name is mixer_music anyways? music FTW!!

Relevant details

• the title, album etc are more clear on what they are rather than metadata, so I agree
• I agree with the MUSICENDED event, but I think it shouldn't be sent with stop(), only when it ends gracefully. if someone needs the same things to happen in both scenarios they can just call the same code in both spots. on the other way around if someone only wanted the event when it ends gracefully, there would be no quick solution, so it should be the default.
• I'd pay money for a correct positioning system, get_pos should get out 😭
• is_playling is incosistent with paused and ended. I personally prefer it to be without is, because "is" is a reserved keyword and my IDE prioritizes it against the attribute on autocompletion. I think it should be playing and that's it.
• Perhaps there could be attributes related to looping, maybe the loop count, or weather the music is looping or not, but this requires discussion.
• The functionality of "unload" should either be contained inside stop(), or be accessible in another way. this is because when you load the music a file handle is created (streamed data) so the OS will hold the file hostage until the handle is released. So if I perhaps want to stop the music and delete the file, I need to close the handle. I propose it to load the music on play(), and unload it on stop(), but I need feedback.

[bilhox]

I think this object should be contained in the pygame.music module. This should be a "modern" api, disconnected from the old api.

I wrote pygame.Music which is an alias to pygame.mixer.music.Music or pygame.music.Music. I should mention it in the original message.

[damusss]

I wrote pygame.Music which is an alias to pygame.mixer.music.Music or pygame.music.Music

I specified the alias should in fact be of pygame.music.Music. Get pygame.mixer_music.Music out of here :kekw:

[bilhox]

This is because this object completely removes the need for mixer_music.

No, it's not the goal. All functions available right now will still update the music object and do what they do. Also I forgot to talk about a really useful function which I'll add to the original message.
And yes, we're aiming for a pygame.music alias 😭 .

[damusss]

No, it's not the goal. All functions available right now will still update the music object and do what they do. Also I forgot to talk about a really useful function which I'll add to the original message. And yes, we're aiming for a pygame.music alias 😭 .

I don't understand, why would someone use mixer_music if this object exists? apart from volume, this looks complete to me. and I think get/set volume should reside in pygame.music

I don't understand another thing, pygame.music would not be an alias, it would be a new module. unless we have different ideas.

[aatle]

My thoughts and observations on the modules (note I haven't really used mixer or music modules in my projects yet):

Nowhere in the docs is the "pygame.mixer_music" module name mentioned specifically; the music module is only publicly exposed via the pygame.mixer.music submodule.

The current implementation where pygame.mixer.music is just an alias to pygame.mixer_music (as opposed to being a real submodule) has import problems. You cannot directly import functions from the music module.

import pygame.mixer.music  # ModuleNotFoundError
from pygame.mixer.music import play  # ModuleNotFoundError

This same problem may occur if you do an alias variable pygame.music for pygame.mixer.music.

Three options for music module:

1. pygame.music is a new alias of pygame.mixer.music, which itself is an alias of the undocumented pygame.mixer_music. Note that the original implementation hadn't just decided to use the name pygame.music in the first place.
2. No new module or module alias is added. The new class Music is placed inside the original pygame.mixer.music module next to all of the module functions. (Similar to pygame.mixer.Sound/Channel, which also have aliases pygame.Sound/Channel.)
3. A new, separate module named pygame.music is created for the new Music class. It's best if this is an independent module with no functions, like pygame.rect.Rect and pygame.surface.Surface. (Here, Music will eventually be available from pygame module directly.) May be a bit confusing (with pygame.mixer.music and pygame.mixer_music existing).

In any case, I agree that Music class should have all of the functionality of the existing music module.

[aatle]

Opinions about implementation:

• Yes, the metadata is most likely better as concrete attributes on this class, mainly because the metadata is not unstructured nor dynamic (see https://pyga.me/docs/ref/music.html#pygame.mixer.music.get_metadata).
• MUSICENDED is definitely more intuitive to use. I think it should indeed trigger for both natural end and manual stop, but also include a stopped bool attribute for differentiating the two. This aligns better with how events are used in general.
• (Nice to have more pythonic properties instead of the pygame get_() set_() accessor methods.) I agree that playing is probably a better property name, as names like is_... are less pythonic IMO.
• position property: note, it will have to understand the different audio formats as set_pos() has different behavior depending on format
• ended property: what is the difference from the playing property?
• paused property: is good. Note, this information is not available in the existing music module I think.
• I dunno how loops should be implemented, maybe with a property for the mixer.music.play() loops parameter
• I think volume should be a property (especially to complete the full functionality). The global volume would automatically be set to local volume inside play(), and both would be kept in line with each other while playing.
• Unloading behavior will be tricky. If Music supports a file object too, then it may not be able to reload. Also, the path object must be stored as an attribute to reload after unloading. While unloaded, the music object does not know certain data about the audio. And can a music object load a different audio file?
• get_music() function seems strange; I say we exclude it for now. It's not guaranteed that the loaded music has an object.
• __init__() should have optional parameters for most attributes, probably keyword only
• How will the legacy functions and behavior be factored into this?

[bilhox]

ended property: what is the difference from the playing property?

Good question, playing is set to False if the music is paused or if it ended. ended however is set to True if the music ended or the music was stopped.

I think volume should be a property (especially to complete the full functionality). The global volume would automatically be set to local volume inside play(), and both would be kept in line with each other while playing.

I don't support this idea for now, as we can't play 2 musics at the same time. It wouldn't make sense in my opinion to have a local and a global volume.

get_music() function seems strange; I say we exclude it for now. It's not guaranteed that the loaded music has an object.

When a music is loaded using a path like before, if you call this function, it's supposed to build you a music object and return it to you. This allows you to edit the music when you don't have access to it in a certain part of your code.

MUSICENDED is definitely more intuitive to use. I think it should indeed trigger for both natural end and manual stop, but also include a stopped bool attribute for differentiating the two. This aligns better with how events are used in general.

I agree.

[damusss]

@aatle @bilhox
playing should be True even if the music is paused, if you care about it you can check for paused aswell, in my opinion, which would remove the need of ended.
regarding the position, @aatle , we don't need to worry about formats as it's handled by Mix_GetMusicPosition. the mixer_music.get_pos implementation does NOT use that function, that's why it's broken.
@bilhox mixer and sounds already have both global volume and local sound volume. it's not that absurd to me, I absolutely think one should be able to customize a music's local volume. it's consistent and it makes sense because maybe one music file is natively louder than another, you'd set one's volume to 1 and the other to 0.8 so they sound the same, while keeping the global volume to 1.

important question to both, why would end event be raised when you stop the music manually? what is the benefit? what if I want my endevent code only to run when it ends gracefully? I don't like "stopped" as an event attribute, it's unintuitive and ugly api-wise in my opinion. what other parts of pygame raise events on manual operation, to justify this?

[bilhox]

mixer and sounds already have both global volume and local sound volume.

This is because with mixer you can play sounds and channels at the same time. It is not the case for musics.

[bilhox]

why would end event be raised when you stop the music manually?

This is the behaviour of ENDEVENT, and I still find this behaviour understandable. If you stop the music, you can't unstop it, and continue from where you stopped. If you want to play the music after it got stopped, you have to restart the music which definitely means, you ended the music playback.

[damusss]

This is because with mixer you can play sounds and channels at the same time. It is not the case for musics.

• it might become the case in the future
• it's still useful, even without. this way you can be sure whenever you play a song you consider too loud (but it's the only one too loud) the volume will be set accordingly. it makes sense for each music to have a custom volume, regardless of the ability to play together.

[damusss]

This is the behaviour of ENDEVENT, and I still find this behaviour understandable. If you stop the music, you can't unstop it, and continue from where you stopped. If you want to play the music after it got stopped, you have to restart the music which definitely means, you ended the music playback.

If this is how ENDEVENT works, then I can understand it, for the sake of consistency. thanks for letting me know!

[bilhox]

we don't need to worry about formats as it's handled by Mix_GetMusicPosition.

While you're mentionning it, IMO I would not, for the sake of maintainability, create a system for the less than 0.5% users using SDL mixer with a version lower than 2.6.0. (You can check the code behind the current implementation, it's a bit terrible) It's not like people are supposed to have this version now, especially when the recent versions of pygame use SDL Mixer 2.8.0 since a pretty long time.

[aatle]

Good question, playing is set to False if the music is paused or if it ended. ended however is set to True if the music ended or the music was stopped.

Hmm, if I understand correctly, ended is derivable from playing and paused. If it's derivable from existing properties, then I don't think it should be implemented, agreeing with damusss. (It can always be added later if it is exceptionally common.)

get_music() function seems strange; I say we exclude it for now. It's not guaranteed that the loaded music has an object.

When a music is loaded using a path like before, if you call this function, it's supposed to build you a music object and return it to you. This allows you to edit the music when you don't have access to it in a certain part of your code.

I still disagree with the get_music() function: if it builds a music object in the call, then it may create a duplicate music object (if user has a separate one), and these cannot be synced. The function would make more sense if Music class were the only way to play music. The existing API will get excluded.
It is the responsibility of the user to make sure their own instance of Music is accessible to code that will edit it.

playing should be True even if the music is paused, if you care about it you can check for paused aswell, in my opinion, which would remove the need of ended.

Though, the same can be said for if playing is False when paused. <- I think this should be the behavior because it aligns with the existing behavior (from docs), and is more intuitive.

[aatle]

Regarding volume, this property will still be useful even when only one music can be played at a time.
First, it completes the core functionality for music, so that music functions don't have to be used with Music objects for this.
Second, it is a more intuitive interface and enables volume to be easily manipulated.

However, the global and local volumes would indeed have to be synced, so perhaps it could be a class property instead, the balanced middle ground. Unfortunately, there may be implementation problems here.

Note: currently, a rounding issue with get_volume()/set_volume() needs to be settled before this can be implemented.

[damusss]

@aatle playing, paused, position, volume would all be properties (some read only) as they use sdl under the hood

hold on, why class property? instance property!

[aatle]

I meant a property that just delegates to global volume, all Music object's volume would be the same.

[damusss]

I meant a property that just delegates to global volume, all Music object's volume would be the same.

what's the utility of that? mixer music volume already exists. it makes more sense as an instance property

[aatle]

What I mean is an instance property that is always the global volume, to mitigate any problems with having both local and global volumes.

[bilhox]

As currently, pausing / stopping is done for any playblack, I would like to remove temporarly paused setter and stop method from the list, you can still get the exact same behaviour using pygame.mixer_music.pause/unpause/stop.

I'll be taking in account the suggestion for volume.

EDIT: same for rewind

[aatle]

Shouldn't the pause, stop, and rewind functionality be available in the Music class?
Yes, it's possible to get the behavior using the global pygame.mixer.music functions, but so is the entirety of the Music class.

Note: paused would be readonly, with a pause() and unpause() method for setting it

[bilhox]

I found a solution for the problems above, therefore, I think when a new music is playing while another (Music or loaded with the old system) is already playing, the other music is stopped (by stopping all playbacks using Mix_HaltMusic), and the new music is played.

Also, I noticed that we can add effects on the music stream using the callback function used by the old system to increment the position. Might be interesting to play with it if you have any famous effects in your mind.

[bilhox]

Updates about the implementation:

• I found a way to reintroduce paused as it was presented, furthermore, when music1 is playing and music2 is set to playback, music1 is to paused. (only works with SDL +2.6.0)
• The following features are only accessable with SDL +2.6.0 : duration (only supported for ogg and mp3), position getter, fadeout (only supported for ogg and mp3).

[damusss]

2 questions:

• why no setter???
• what happens when an unsupported format tries to get the duration? (what kind of error?)
• how performat is the duration getting? what kind of sdl function does it use?

[bilhox]

why no setter???

What setters ?

what happens when an unsupported format tries to get the duration? (what kind of error?)
how performat is the duration getting? what kind of sdl function does it use?

I use Mix_MusicDuration (SDL 2.6.0 function) for this, for wav and midi files, it returns a random const value (not even -1.0 for example).

[damusss]

of course, the position setter!
you didn't tell me what kind of other errors are raised for other unsupported formats, unless you already covered them all, in that case it's ok if it's mentioned in the docs

[bilhox]

the position setter

Ah this one, yes I'm curently reimplementing it as I found a solution for it as well.

[aatle]

So, can you give a summary of all of the properties and methods, and which may raise pygame.error for unsupported formats?

[bilhox]

So, can you give a summary of all of the properties and methods, and which may raise pygame.error for unsupported formats?

It'll not raise an error, the user didn't make a mistake so it doesn't make sense. Therefore, they will get -1.0 if it's not supported.
I'm pretty much done for a first version, so except a PR soon.

[aatle]

Therefore, they will get -1.0 if it's not supported.

Okay. What about property setters though? Are they guaranteed to work?