-
Notifications
You must be signed in to change notification settings - Fork 617
getSessionId returns integer but uses SessionId allocation strategy enum as return value. #2351
Description
The AudioStreamBase::getSessionId method currently returns a SessionId enum:
enum SessionId { /** * Do not allocate a session ID. * Effects cannot be used with this stream. * Default. */ None = -1, // AAUDIO_SESSION_ID_NONE /** * Allocate a session ID that can be used to attach and control * effects using the Java AudioEffects API. * Note that the use of this flag may result in higher latency. * * Note that this matches the value of AudioManager.AUDIO_SESSION_ID_GENERATE. */ Allocate = 0, // AAUDIO_SESSION_ID_ALLOCATE };
And the documentation for AudioStreamBase::getSessionId states:
@return the stream's session ID allocation strategy (None or Allocate).
However, the implementation actually returns the real session ID integer when one has been allocated. For example, in AudioStreamAAudio.cpp, the result of the AAudio call is cast directly to the enum:
mSessionId = static_cast<SessionId>(mLibLoader->stream_getSessionId(mAAudioStream));According to the AAudio documentation, AAudioStream_getSessionId returns an aaudio_session_id_t, which is defined as an int32_t. While AAUDIO_SESSION_ID_NONE (-1) and AAUDIO_SESSION_ID_ALLOCATE (0) are valid constants, a successfully allocated session ID is always a positive and nonzero integer.
Why this is an issue:
- Inaccurate Documentation: The comment claims to return a "strategy" (
NoneorAllocate), but it actually returns the resulting ID (e.g., 42). - Confusing Type System: The
SessionIdenum only explicitly definesNone = -1andAllocate = 0. Using it to store values outside this range (like a raw session ID) is misleading for developers using the API. - API Consistency: In
AudioStreamBuilder.h, thesetSessionIdmethod documentation correctly identifies that it can take an allocated ID. This acknowledges that theSessionIdtype is being overloaded to act as a raw integer:@param sessionId an allocated sessionID or SessionId::Allocate
Proposed Solution:
- Update the documentation for the
SessionIdenum and thegetSessionId()method to explicitly state that positive values represent the actual allocated session ID. - Consider providing a clearer way to distinguish between the allocation request and the resulting ID, perhaps by returning a raw
int32_tfor the ID and keeping the enum only for builder configuration. - At a minimum, the
getSessionId()comment should be corrected to:@return the actual session ID, SessionId::None, or SessionId::Allocate.