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

Move logging callback into an OpenAL extension #939

Conversation

MathiusD
Copy link
Contributor

The aim here is to enable access to a logging callback through an OpenAL extension (here ALC_SOFT_logging_events).

Logging callback has already been implemented here: #862.

The only difference with the previously implemented callback is the following modifications:

The log level is now represented by an ALCenum
Two new parameters are added:

  • A null terminated string containing the name of the entity issuing the log.
  • The size of the previous parameter

This string, representing the emitter, can be null, if the implementation cannot provide an emitter section. The emitter represents the impacted section, aka the name of the extension, name of method, or name of specification from which the log comes.

@MathiusD MathiusD changed the title Move logging collback into an OpenAL extension Move logging callback into an OpenAL extension Nov 27, 2023
@MathiusD MathiusD force-pushed the feature/move_logging_callback_in_dedicated_ext branch from d998cc1 to 2574409 Compare November 27, 2023 21:42
@kcat
Copy link
Owner

kcat commented Nov 28, 2023

I'm not keen on making this an extension since it's dealing with implementation details of what the library's doing, things which can change. The app isn't supposed to gleam any details from it, it's just a way to include OpenAL Soft's logging messages in the app's own log, to avoid the user having to mess with the ALSOFT_LOGLEVEL/ALSOFT_LOGFILE env var stuff.

There is instead the AL_EXT_debug API for the app to get OpenAL-related messages (primarily just API errors for now, with some portability warnings).

@MathiusD
Copy link
Contributor Author

MathiusD commented Nov 28, 2023

Okay, I understand (and to tell the truth, I hadn't explored AL_EXT_DEBUG enough ^^' and i'd like to use alsoft non extension exports (Without include alc/inprogrext.h because following contains many symbols for unreleased extension and ABI) so i have submitted current PR)

However, I'd like to take this opportunity to ask another question: Is it desirable to expose these implementation-dependent methods (i.e. all those in inprogrext.h that don't concern an extension or an ABI) in a public include? (For example, in include/SOFT/api.h) (An example of what I mean here : master...mathiusd:feature/move_non_standard_export_into_dedicated_header) (I ask because it allows anyone to manage includes more finely).

@MathiusD MathiusD closed this Nov 28, 2023
@kcat
Copy link
Owner

kcat commented Nov 28, 2023

Functions that don't have AL_API or ALC_API aren't exported from the library, so a function declaration in a public header isn't too appropriate. I also prefer to limit the number of exported non-essential functions (non-standard, and what an app won't require to work).

Ideally I'd like a way to export a function so that it can be retrieved with dlsym/GetProcAddress, but not used directly to get at link-time. There doesn't seem to be a way to do that though, and putting it in a public header would make it easy to make the function required at run-time.

@MathiusD
Copy link
Contributor Author

Okay, thanks a lot for the explanation!

@MathiusD MathiusD deleted the feature/move_logging_callback_in_dedicated_ext branch November 28, 2023 21:50
@MathiusD MathiusD restored the feature/move_logging_callback_in_dedicated_ext branch November 28, 2023 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants