-
-
Notifications
You must be signed in to change notification settings - Fork 773
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
Ability to read comments/metadata from sound files #1656
base: master
Are you sure you want to change the base?
Conversation
Great work, thank you! Some comments. We're trying to phase out Python extensions added in direct. They're only available when direct is loaded. Prefer a C++ interface if at all possible, or a C++-implemented extension method (see _ext.h and _ext.cxx files) if nothing else is possible. Doing it in C++ would mean making sequential access methods (which means you could parse it into a SimpleHashMap, that supports both key-based and indexed access) and exposing those to Python with MAKE_MAP_PROPERTY - do a grep for that to search for examples. We can keep the C++ methods making up the map-property as unpublished, so that the property is the only way to access it from Python. An alternative is making an AudioTags class which exposes a mapping interface (overriding I don't think that we should have this interface on both AudioSound and MovieAudioCursor. Intuitively, MovieAudio should be the place for this, but it seems some implementations don't actually open the sound file until the cursor is created, so MovieAudioCursor sounds like the place to get it. Is there a reason to also have it on AudioSound? Would it be interesting to give thought to standardizing the keys between different formats, so that you can use the same code to access the title of an .mp3 file as for an .ogg file, via a named enum or named getters, or do you think the original keys should be kept? Feel free to ping me on Discord for more discussion. |
Implemented this. An interesting consequence now is that you can no longer "parse all comments", only "comments with a given named tag". Not sure whether this is a significant issue or not. I could still expose the string as something like
Decided against it since it felt overkill.
AudioSound is returned from loader, so I think it only makes sense. Other 2 having it is implementation detail because Panda3D actually has two levels of abstraction you have to go through.
I could implement a series of getters like |
Issue description
This PR allows loading metadata/comments/tags from sound files and exposes those to the programmer. Example code:
Solution description
Metadata is exposed through two virtual methods, one on the AudioCursor, and another one on the AudioSound, linked through some binding code in AudioManager.
We have 3 sound managers, FMod, Null and OpenAL. FMod is way too encapsulated to do anything, and Null is Null, so those 2 managers return an empty string when the comment is read. For OpenAL, the method depends on the underlying AudioCursor class.
There are seven types of AudioCursor: FFmpeg, Opus, Flac, Wav, Vorbis, Microphone and Userdata. Only three of these currently have an implementation:
vector<string>
.getComment()
returns an empty string here also.Due to an interrogate limitation, the published C++ functions returnno longer the case, see belowstring
which is obtained by joining the vector of comments with newlines. Not sure if there's a better way.I've tested the implementation on Libvorbis which seems to work correctly, did not test FFmpeg and Opus (besides ensuring the engine compiles and can be imported without ABI errors) since there is no documentation on how to force Panda to use a specific cursor loader.
Checklist
I have done my best to ensure that…