Closed Bug 837573 Opened 9 years ago Closed 9 years ago

[Audio]No music sound output after A2dp link disconnected

Categories

(Firefox OS Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: shawnjohnjr, Assigned: rlin)

References

Details

Attachments

(1 file, 3 obsolete files)

This bug tracking is not for Firefox V1.
But for A2DP implementation.

After calling AudioSystem::setDeviceConnectionState and set AUDIO_DEVICE_OUT_BLUETOOTH_A2DP as AUDIO_POLICY_DEVICE_STATE_UNAVAILABLE. No sound can be heard.
log pattern as following.

02-04 03:33:49.492   117   538 W AudioFlinger: Thread AudioOut_7 cannot connect to the power manager service
02-04 03:33:49.602   117   478 V AudioPolicyManager: getDeviceForStrategy() strategy 0, device 2
02-04 03:33:49.602   117   478 V AudioPolicyManager: getDeviceForStrategy() strategy 1, device 1
02-04 03:33:49.602   117   478 V AudioPolicyManager: getDeviceForStrategy() strategy 2, device 2
02-04 03:33:49.602   117   478 V AudioPolicyManager: getDeviceForStrategy() strategy 0, device 2
02-04 03:33:49.602   117   478 V AudioPolicyManager: getDeviceForStrategy() strategy 3, device 2
02-04 03:33:49.602   117   478 V AudioPolicyManager: getDeviceForStrategy() strategy 4, device 2
02-04 03:33:49.602   117   478 V AudioPolicyManager: setOutputDevice() output 1 device 0 delayMs 0
02-04 03:33:49.602   117   478 V AudioPolicyManager: setOutputDevice() setting same device 0 or null device for output 1
02-04 03:33:49.602   117   222 V qcom_audio_policy_hal: audio_io_handle_t android_audio_legacy::ap_get_output(audio_policy*, audio_stream_type_t, uint32_t, uint32_t, uint32_t, audio_policy_output_flags_t): tid 222
02-04 03:33:49.602   117   222 V AudioPolicyManager: getDeviceForStrategy() from cache strategy 0, device 2
02-04 03:33:49.602   117   474 V qcom_audio_policy_hal: audio_io_handle_t android_audio_legacy::ap_get_output(audio_policy*, audio_stream_type_t, uint32_t, uint32_t, uint32_t, audio_policy_output_flags_t): tid 474
02-04 03:33:49.602   117   474 V AudioPolicyManager: getDeviceForStrategy() from cache strategy 0, device 2
02-04 03:33:49.612   117   117 E AudioFlinger: unknown output thread
02-04 03:33:49.612   458   542 E AudioTrack: AudioFlinger could not create track, status: -22
02-04 03:33:49.612   458   542 W AudioTrack: restoreTrack_l() failed status -22
02-04 03:33:49.612   458   542 W AudioTrack: restoreTrack_l() error -22 TID 542
02-04 03:33:49.612   458   542 W AudioTrack: obtainBuffer create Track error -22
It seems sydney_audio channel use wrong definition value of AudioSystem.h
        CHANNEL_OUT_FRONT_LEFT = 0x4,
        CHANNEL_OUT_FRONT_RIGHT = 0x8,
        CHANNEL_OUT_STEREO = (CHANNEL_OUT_FRONT_LEFT | CHANNEL_OUT_FRONT_RIGHT),  = 0x3
But android media system's definition is 
        CHANNEL_OUT_FRONT_LEFT            = 0x1,
        CHANNEL_OUT_FRONT_RIGHT           = 0x2,
        CHANNEL_OUT_STEREO = (CHANNEL_OUT_FRONT_LEFT | CHANNEL_OUT_FRONT_RIGHT), = 0xc

This function AudioPolicyManagerBase::getOutput would return 0 if the channel type isn't match MONO /STEREO.
Attached patch patch 1 (obsolete) — Splinter Review
Assignee: nobody → rlin
Attachment #710547 - Flags: review?(chris.double)
Comment on attachment 710547 [details] [diff] [review]
patch 1

Passing review to Matthew who is more familiar with libsydneyaudio.
Attachment #710547 - Flags: review?(chris.double) → review?(kinetik)
Comment on attachment 710547 [details] [diff] [review]
patch 1

Review of attachment 710547 [details] [diff] [review]:
-----------------------------------------------------------------

That's a nasty trap.  Thanks!

We're using AudioStream::* constants elsewhere in sydney_audio_gonk.cpp too.  The rest of the ones I checked had matching values, but since we've been bitten once, it's probably best to fix them all at the same time.  Would you mind doing that?
Attachment #710547 - Flags: review?(kinetik) → feedback+
OK, I will check and sync those values to ICS and update patch for this.
I found this cause by using the channel mask definition on AudioFormat.java and the value is "bit left shift 2" as the value in audio.h
The google do the trick on this jni code. 
android_media_AudioTrack.cpp
===
    // Java channel masks don't map directly to the native definition, but it's a simple shift
    // to skip the two deprecated channel configurations "default" and "mono".	
    uint32_t nativeChannelMask = ((uint32_t)javaChannelMask) >> 2;
==
So If sydney_audio_gonk.cpp use the audioTrack.cpp directly, The channel mask should use the value in audio.h
Attached patch patch1 (obsolete) — Splinter Review
Remove duplicate definition & correct values
Attachment #710547 - Attachment is obsolete: true
Attachment #715067 - Flags: review?(kinetik)
Are those changes synchronizing AudioSystem.h with the Android version, or making new modifications? We don't want to carry our own changes to those headers as they should be identical to the system copies... if sydneyaudio is using the wrong constants, the changes should be made in sydneyaudio.
Hi Matthew, 
Those modify values are sync to the 
system/core/include/system/audio.h
typedef enum {
    /* output channels */
    AUDIO_CHANNEL_OUT_FRONT_LEFT            = 0x1,
    AUDIO_CHANNEL_OUT_FRONT_RIGHT           = 0x2,
....
    /* input channels */
    AUDIO_CHANNEL_IN_LEFT            = 0x4,
    AUDIO_CHANNEL_IN_RIGHT           = 0x8,
....
Okay, thanks.  I think we should keep AudioSystem.h identical to the upstream version, and make the changes in sydneyaudio to use the correct constants.

Your original patch was on the right track, I just wanted to make sure there were not any additional constants used in sydneyaudio that were using the Java versions instead of the native versions (e.g. AudioSystem::CHANNEL_OUT_MONO vs AUDIO_CHANNEL_OUT_MONO).
Attached patch patch 2 (obsolete) — Splinter Review
I just checked and found no more source code use the java constant in sydneyaudio.
Attachment #715067 - Attachment is obsolete: true
Attachment #715067 - Flags: review?(kinetik)
Attachment #715082 - Flags: review?(kinetik)
Comment on attachment 715082 [details] [diff] [review]
patch 2

Thanks!
Attachment #715082 - Flags: review?(kinetik) → review+
Whiteboard: checkin-needed
Attachment #715082 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/725e172de358
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
blocking-b2g: --- → koi?
Unclear why this was nommed for koi, this landed long back based on comment #16 and should be there in 1.2.
blocking-b2g: koi? → ---
You need to log in before you can comment on or make changes to this bug.