Android mediaPlayer does not follow observer requirement...

[wjennings]

In MediaPlayerInterface.h, there is this line:

Callbacks should not be made on the caller's thread prior to returning from a call.

However, in AndroidSLESMediaPlayer.cpp, all of the m_observer callbacks happen within the caller's thread before returning form the call. Example:

bool AndroidSLESMediaPlayer::play(SourceId id) {
    ACSDK_DEBUG7(LX(__func__).d("requestId", id));

    std::lock_guard<std::mutex> lock{m_operationMutex};
    if (id == m_sourceId) {
        SLuint32 state;
        auto result = (*m_player)->GetPlayState(m_player, &state);
        if (result != SL_RESULT_SUCCESS) {
            ACSDK_ERROR(LX("playFailed").d("reason", "getPlayStateFailed").d("result", result));
            return false;
        }

        if (SL_PLAYSTATE_STOPPED == state) {
            // set the player's state to playing
            result = (*m_player)->SetPlayState(m_player, SL_PLAYSTATE_PLAYING);
            if (result != SL_RESULT_SUCCESS) {
                ACSDK_ERROR(LX("playFailed").d("reason", "setPlayStateFailed").d("result", result).d("id", id));
                return false;
            }

            if (m_observer) {
                m_observer->onPlaybackStarted(id);
            }

            return true;
        }
        ACSDK_ERROR(LX("playFailed").d("reason", "invalidState").d("requestId", id).d("state", state));
    } else {
        ACSDK_ERROR(LX("playFailed").d("reason", "invalidId").d("requestId", id).d("currentId", m_sourceId));
    }
    return false;
}

Is this an issue we should be concerned with?

[kclchan]

Hi @wjennings, thanks for bringing this to our attention. Are you seeing actual problems with the AndroidSLESMediaPlayer, or is this mostly a concern you see that you want clarification? Thanks!

[wjennings]

No problems with it, but I noticed it and it just bothered me it wasn't following the requirements because I assumed it could cause an issue (and because, when I wrote our own media player, I had to do extra work to follow these requirements ;)

[kclchan]

Thanks for letting us know about this. The documentation has been updated.