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.

Sign up for GitHub

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

Jump to bottom

Replace OpenAL audio device for Miniaudio backend #1339

Merged
merged 2 commits into from
Oct 9, 2024
Merged

Replace OpenAL audio device for Miniaudio backend #1339

merged 2 commits into from
Oct 9, 2024

Conversation

diogomsmiranda
Copy link
Contributor

@diogomsmiranda diogomsmiranda commented Oct 3, 2024 •
edited
Loading

Description

This PR is one of many in #1004, it replaces the OpenAL audio backend with the miniaudio.h backend.

Closes #1005.

Checklist

  • Self-review changes.
  • Add entry to the changelog's unreleased section.

@diogomsmiranda diogomsmiranda added this to the 0.4 milestone Oct 3, 2024
@diogomsmiranda diogomsmiranda requested review from RiscadoA and a team as code owners October 3, 2024 22:39
@diogomsmiranda diogomsmiranda linked an issue Oct 3, 2024 that may be closed by this pull request
Replace OpenAL audio device by another backend #1005
Closed
@github-actions github-actions bot added the S-Needs-Design Demands some time designing an implementation label Oct 3, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 3, 2024 •
edited
Loading

PR Preview Action v1.4.8
🚀 Deployed preview to https://GameDevTecnico.github.io/cubos/preview/pr-1339/
on branch gh-pages at 2024-10-09 22:31 UTC

github-actions[bot]
github-actions bot reviewed Oct 3, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
@codecov Codecov
Copy link

codecov bot commented Oct 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 147 lines in your changes missing coverage. Please review.

Project coverage is 40.20%. Comparing base (4ebb4a3) to head (646ad5f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
core/src/al/miniaudio_context.cpp 0.00% 135 Missing ⚠️
core/include/cubos/core/al/audio_context.hpp 0.00% 10 Missing ⚠️
core/src/al/audio_context.cpp 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1339      +/-   ##
==========================================
- Coverage   40.37%   40.20%   -0.17%     
==========================================
  Files         432      432              
  Lines       30745    30869     +124     
==========================================
  Hits        12412    12412              
- Misses      18333    18457     +124

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@diogomsmiranda diogomsmiranda changed the title feat(audio): replace OpenAL audio device for Miniaudio backend (#1005) Replace OpenAL audio device for Miniaudio backend Oct 3, 2024
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch from 7193ef1 to 3499340 Compare October 3, 2024 23:11
github-actions[bot]
github-actions bot reviewed Oct 3, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/include/cubos/core/al/miniaudio_context.hpp Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Show resolved Hide resolved
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch from 3499340 to 38ae560 Compare October 3, 2024 23:15
github-actions[bot]
github-actions bot reviewed Oct 3, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/src/al/miniaudio_context.cpp Show resolved Hide resolved
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch from 38ae560 to 469b320 Compare October 3, 2024 23:22
@RiscadoA RiscadoA mentioned this pull request Oct 4, 2024
Open
2 tasks
@RiscadoA RiscadoA removed the S-Needs-Design Demands some time designing an implementation label Oct 4, 2024
RiscadoA
RiscadoA approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@RiscadoA RiscadoA left a comment

Choose a reason for hiding this comment

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

Thank you both for working on this! LGTM after resolving the comments I've just added 😌

core/include/cubos/core/al/audio_context.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/al/audio_context.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/al/audio_context.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/al/audio_context.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/al/audio_context.hpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/include/cubos/core/al/audio_context.hpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch from 469b320 to 7560baf Compare October 4, 2024 13:21
@github-actions github-actions bot added the S-Needs-Design Demands some time designing an implementation label Oct 4, 2024
github-actions[bot]
github-actions bot reviewed Oct 4, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
core/src/al/miniaudio_context.cpp Outdated Show resolved Hide resolved
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch 2 times, most recently from 659921e to 30d7741 Compare October 4, 2024 13:45
GalaxyCrush
GalaxyCrush approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@GalaxyCrush GalaxyCrush left a comment

Choose a reason for hiding this comment

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

LGTM!

@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch from 30d7741 to 461b647 Compare October 6, 2024 14:25
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch 9 times, most recently from 2f43f95 to e1d6253 Compare October 7, 2024 21:05
@RiscadoA RiscadoA force-pushed the 1005-replace-openal-audio-device-by-another-backend branch 3 times, most recently from 7823314 to b8c58d2 Compare October 9, 2024 12:41
@diogomsmiranda diogomsmiranda enabled auto-merge (rebase) October 9, 2024 22:30
diogomsmiranda and others added 2 commits October 9, 2024 23:30
@diogomsmiranda @Dageus
feat(audio): replace OpenAL audio backend for Miniaudio backend (#1005)
1fe986a 
Co-Authored-By: João Miguel Nogueira <[email protected]>
@RiscadoA @diogomsmiranda
fix(ci): stop using homebrew clang in macOS action
646ad5f 
We can't compile Miniaudio using Homebrew clang for some reason.
We had the same exact compiling error as reported in RustAudio/coreaudio-sys#96
@diogomsmiranda diogomsmiranda force-pushed the 1005-replace-openal-audio-device-by-another-backend branch from b8c58d2 to 646ad5f Compare October 9, 2024 22:30
@diogomsmiranda diogomsmiranda merged commit 86c062d into main Oct 9, 2024
9 checks passed
@diogomsmiranda diogomsmiranda deleted the 1005-replace-openal-audio-device-by-another-backend branch October 9, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@RiscadoA RiscadoA RiscadoA approved these changes

@GalaxyCrush GalaxyCrush GalaxyCrush approved these changes

@Dageus Dageus Dageus approved these changes

Assignees

@diogomsmiranda diogomsmiranda

@Dageus Dageus

Labels
A-Core B-Audio S-Needs-Design Demands some time designing an implementation
Projects
None yet
Milestone
0.4
Development

Successfully merging this pull request may close these issues.

Replace OpenAL audio device by another backend
4 participants
@diogomsmiranda @RiscadoA @GalaxyCrush @Dageus