Skip to content

getSessionId returns integer but uses SessionId allocation strategy enum as return value. #2351

@vikwil

Description

@vikwil

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:

  1. Inaccurate Documentation: The comment claims to return a "strategy" (None or Allocate), but it actually returns the resulting ID (e.g., 42).
  2. Confusing Type System: The SessionId enum only explicitly defines None = -1 and Allocate = 0. Using it to store values outside this range (like a raw session ID) is misleading for developers using the API.
  3. API Consistency: In AudioStreamBuilder.h, the setSessionId method documentation correctly identifies that it can take an allocated ID. This acknowledges that the SessionId type is being overloaded to act as a raw integer:

    @param sessionId an allocated sessionID or SessionId::Allocate

Proposed Solution:

  • Update the documentation for the SessionId enum and the getSessionId() 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_t for 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.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions