Skip to content
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

No longer possible to load PackedByteArray into AudioStreamOGGVorbis #61091

Closed
Tracked by #76797
Chlipouni opened this issue May 16, 2022 · 14 comments · Fixed by #78084
Closed
Tracked by #76797

No longer possible to load PackedByteArray into AudioStreamOGGVorbis #61091

Chlipouni opened this issue May 16, 2022 · 14 comments · Fixed by #78084

Comments

@Chlipouni
Copy link

Godot version

Godot_v4.0-alpha8_win64

System information

Windows 10

Issue description

In Godot 3.5, the AudioStreamOGGVorbis class contains a data property which accepts a PackedByteArray value.
In Godot 4.0 alpha 8, it is replaced by the OGGPacketSequence property.
I didn't find a function to convert the PackedByteArray to an OGGPacketSequence.
No more information in the Godot 4.0 documentation.

Steps to reproduce

Minimal reproduction project

No response

@Chaosus Chaosus added this to the 4.0 milestone May 16, 2022
@Chaosus Chaosus moved this to To Assess in 4.x Priority Issues May 16, 2022
@Calinou Calinou removed the bug label May 16, 2022
@Calinou
Copy link
Member

Calinou commented May 16, 2022

This is likely related to AudioServer refactoring. cc @ellenhp

@ellenhp
Copy link
Contributor

ellenhp commented May 17, 2022

This is because of the stb_vorbis replacement. I didn't realize we needed to support runtime loading of oggs.

@nickpolet
Copy link

@ellenhp Has there been any update on adding this functionality back in? It's really quite useful for editor tools/plugin that pull in audio from various sources.

@YuriSizov YuriSizov changed the title No more possible to load PackedByteArray into AudioStreamOGGVorbis No longer possible to load PackedByteArray into AudioStreamOGGVorbis Sep 12, 2022
@YuriSizov YuriSizov moved this from To Assess to Todo in 4.x Priority Issues Sep 12, 2022
@mattkanwisher
Copy link

mattkanwisher commented Nov 2, 2022

We are running into same issue in our music game. Any chance this will be fixed?

Would it be possible to just expose "import_ogg_vorbis" function from C++ to GDscript and that would solve most of the issues.

@jotson
Copy link
Contributor

jotson commented May 17, 2023

Thanks for your work on this, everyone. I just wanted to add my use case so that you have a better idea of why this is useful to me.

I'm writing a Twitch bot (a Godot app) that connects to chat and plays sounds when certain events happen. The sounds that are played change frequently so I just have a system where the sounds are dynamically loaded from a specific user:// directory. This allows me to change the sounds without needing to specifically add them to the Godot project or rebuild the app. If I released this to the public, I wouldn't want to include the sounds with the app. I would want users to be able to add their own sounds and configure them.

For now I'm keeping the app in 3.5 but I would like to upgrade it 4.x eventually. Thanks, folks!

@Calinou Calinou modified the milestones: 4.1, 4.x May 17, 2023
@GrowtopiaFli
Copy link

Good morning/afternoon/evening!

I have been frustrated about this whole issue myself since I'm migrating my project from Godot 3.5.1 to Godot 4.0.3 so I decided to create a GDExtension Library. I hope that it is ok to share my library here with you and I also hope that you will be able to use and understand it. As of now, it only supports Windows, but you can compile it with your Linux and MacOS devices with a bit of research and by using scons. I'll update the instructions in the future to give information about compiling.

OggLoader

@AThousandShips
Copy link
Member

I haven't tried it, and don't know how it differs from the way it worked in 3.x, but you have this property: OggPacketSequence.packet_data

@GrowtopiaFli
Copy link

GrowtopiaFli commented Jun 12, 2023

I haven't tried it, and don't know how it differs from the way it worked in 3.x, but you have this property: OggPacketSequence.packet_data

Two properties matter: granule_positions and packet_data
An OGG file is decoded into a Bitstream or Packetstream (whatever you want to call it) and it contains multiple pages. Each page contains a granule position and packet list. the granule position is an int64 which is why the granule_positions property is a list of int64s and the packet list or whatever is a list of numbers which is why packet_data is a 3 dimensional array.

Here's the difference:
In the AudioStreamOggVorbis class back in Godot 3, there was a property named data which is a PoolByteArray. Now in Godot 4, it has been renamed to packet_sequence and it assigned a new class named OggPacketSequence which contains the rawest information about oggs. (granule_positions and packet_data)

@AThousandShips

This comment was marked as outdated.

@ellenhp
Copy link
Contributor

ellenhp commented Jun 12, 2023

each comment pings everyone that's subscribed to the issue or the repository

Apologies for this, but I do feel like I want to chime in just to apologize for breaking this, I didn't realize it was such a common use case, or, truthfully, a use case at all. The stb_vorbis maintainer suggested that Godot was too high-profile a project to use stb_vorbis, and due to undefined maximum ogg/vorbis packet sizes, libogg sometimes has to make allocations on the audio thread which is against our current policy. So the choices are a packet ring buffer or similar data structure (complex) and doing this, which is, also complex. I think I may have picked the wrong option, but there weren't really many good options.

@GrowtopiaFli no pressure, but if you've gone to the trouble of learning about container formats and granule positions and whatnot, would you mind looking at the seek code and flagging obvious bugs, if you find any? (new issue, tag me). I have stared at that binary search until my eyes glaze over without anything jumping out at me, and there are a few popping bugs related to the fact that we don't have sample-accurate seeks.

@GrowtopiaFli
Copy link

GrowtopiaFli commented Jun 12, 2023

each comment pings everyone that's subscribed to the issue or the repository

Apologies for this, but I do feel like I want to chime in just to apologize for breaking this, I didn't realize it was such a common use case, or, truthfully, a use case at all. The stb_vorbis maintainer suggested that Godot was too high-profile a project to use stb_vorbis, and due to undefined maximum ogg/vorbis packet sizes, libogg sometimes has to make allocations on the audio thread which is against our current policy. So the choices are a packet ring buffer or similar data structure (complex) and doing this, which is, also complex. I think I may have picked the wrong option, but there weren't really many good options.

@GrowtopiaFli no pressure, but if you've gone to the trouble of learning about container formats and granule positions and whatnot, would you mind looking at the seek code and flagging obvious bugs, if you find any? (new issue, tag me). I have stared at that binary search until my eyes glaze over without anything jumping out at me, and there are a few popping bugs related to the fact that we don't have sample-accurate seeks.

You didn't break anything mate, you just changed the structure and way of loading OGG files so yknow it's good.

Also yeah, I will notify you about any sort of bugs in the seek code. Which seek code exactly?

@GrowtopiaFli
Copy link

each comment pings everyone that's subscribed to the issue or the repository

Apologies for this, but I do feel like I want to chime in just to apologize for breaking this, I didn't realize it was such a common use case, or, truthfully, a use case at all. The stb_vorbis maintainer suggested that Godot was too high-profile a project to use stb_vorbis, and due to undefined maximum ogg/vorbis packet sizes, libogg sometimes has to make allocations on the audio thread which is against our current policy. So the choices are a packet ring buffer or similar data structure (complex) and doing this, which is, also complex. I think I may have picked the wrong option, but there weren't really many good options.

@GrowtopiaFli no pressure, but if you've gone to the trouble of learning about container formats and granule positions and whatnot, would you mind looking at the seek code and flagging obvious bugs, if you find any? (new issue, tag me). I have stared at that binary search until my eyes glaze over without anything jumping out at me, and there are a few popping bugs related to the fact that we don't have sample-accurate seeks.

@ellenhp Also just a side question, are you planning to add a feature for getting the comments of oggs (artist name, title name, whatever)

@vitorventurin
Copy link

Hello, does anyone have any update about this? @GrowtopiaFli your lib does not seem to work. I opened your demo folder and there is incomplete code "OggLo".

@GrowtopiaFli
Copy link

Hello, does anyone have any update about this? @GrowtopiaFli your lib does not seem to work. I opened your demo folder and there is incomplete code "OggLo".

Yeah it's unfortunately no longer maintained and only works on version 4.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project