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

Implement loading OGG files from buffer and file path #78084

Merged
merged 1 commit into from
Jul 16, 2023

Conversation

fire
Copy link
Member

@fire fire commented Jun 10, 2023

Remake of #68540

Test Plan:

Tested that ordinary OGGs load.

Load from buffer needs testing.

Assistance Log

audio-coding.md

Production edit: Closes #61091

@fire fire requested a review from a team as a code owner June 10, 2023 15:53
@fire fire marked this pull request as draft June 10, 2023 15:54
@fire fire force-pushed the load-ogg-bytes-remake branch 3 times, most recently from 90c6754 to b0b7c39 Compare June 10, 2023 16:47
@Calinou Calinou added this to the 4.x milestone Jun 10, 2023
@fire fire force-pushed the load-ogg-bytes-remake branch 3 times, most recently from ad87586 to 43d85fd Compare June 10, 2023 17:09
@fire fire marked this pull request as ready for review June 10, 2023 17:37
@fire fire modified the milestones: 4.x, 4.2 Jun 10, 2023
@GrowtopiaFli
Copy link

Legend

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some style comments, but the implementation looks ok.

@fire
Copy link
Member Author

fire commented Jul 5, 2023

I will be slow modifying this.

@YuriSizov YuriSizov changed the title Load OGGs from file system Implement loading OGG files from buffer and file path Jul 7, 2023
@YuriSizov
Copy link
Contributor

from_filesystem is a weird naming convention. Shouldn't it be from_path?

@KoBeWi
Copy link
Member

KoBeWi commented Jul 7, 2023

Could be load_from_file like in Image.

@YuriSizov
Copy link
Contributor

@fire Any chance you could update this soon?

@fire
Copy link
Member Author

fire commented Jul 15, 2023

Will update soon.

@fire fire force-pushed the load-ogg-bytes-remake branch 5 times, most recently from 7d1e51a to 10b6011 Compare July 15, 2023 01:39
@fire

This comment was marked as outdated.

@fire fire force-pushed the load-ogg-bytes-remake branch 2 times, most recently from 7c12dcf to edcec53 Compare July 15, 2023 01:58
modules/vorbis/audio_stream_ogg_vorbis.cpp Outdated Show resolved Hide resolved
modules/vorbis/audio_stream_ogg_vorbis.cpp Outdated Show resolved Hide resolved
modules/vorbis/audio_stream_ogg_vorbis.h Outdated Show resolved Hide resolved
modules/vorbis/doc_classes/AudioStreamOggVorbis.xml Outdated Show resolved Hide resolved
@YuriSizov YuriSizov merged commit 7bc8a52 into godotengine:master Jul 16, 2023
13 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@fire fire deleted the load-ogg-bytes-remake branch July 16, 2023 13:24
@Overvault-64
Copy link

@YuriSizov Should we expect this to be featured in 4.1.2?

@YuriSizov
Copy link
Contributor

@Overvault-64 You should expect this in 4.2. We normally don't include new features in maintenances releases (4.1.x).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No longer possible to load PackedByteArray into AudioStreamOGGVorbis
9 participants