1007552 - [Madai] 'publicnotification' channel should be fixed for camera shutter

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Seil Park]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140314220517

Steps to reproduce:

changed camera shutter to 'publicnotification' channel

Sounds.prototype.createAudio = function(url) {
  var audio = new Audio(url);
  // MADAI ONLY. Shutter shound should be always played
  audio.mozAudioChannelType = 'publicnotification'; // notification
  return audio;
};

I checked stream type in AudioTrack.cpp


Actual results:

Currently AudioTrack`s streamType is AUDIO_STREAM_SYSTEM

01-12 19:24:45.959 V/AudioTrack(24964): sampleRate 48000, channelMask 0x1, format 1
01-12 19:24:45.959 V/AudioTrack(24964): streamType 1
01-12 19:24:45.959 V/AudioTrack(24964): set() streamType 1 frameCount 0 flags 0004



Expected results:

The shutter sound should be enforced for carrier/country spec. 
If audio chnnel is 'publicnotification' , it should call 'AUDIO_STREAM_ENFORCED_AUDIBLE' stream type. 

/* Audio stream types */
typedef enum {
    AUDIO_STREAM_DEFAULT          = -1,
    AUDIO_STREAM_VOICE_CALL       = 0,
    AUDIO_STREAM_SYSTEM           = 1,
    AUDIO_STREAM_RING             = 2,
    AUDIO_STREAM_MUSIC            = 3,
    AUDIO_STREAM_ALARM            = 4,
    AUDIO_STREAM_NOTIFICATION     = 5,
    AUDIO_STREAM_BLUETOOTH_SCO    = 6,
    AUDIO_STREAM_ENFORCED_AUDIBLE = 7, /* Sounds that cannot be muted by user and must be routed to speaker */
    AUDIO_STREAM_DTMF             = 8,
    AUDIO_STREAM_TTS              = 9,
    AUDIO_STREAM_INCALL_MUSIC     = 10,
    AUDIO_STREAM_CNT,
    AUDIO_STREAM_MAX              = AUDIO_STREAM_CNT - 1,
} audio_stream_type_t;

Comment 1

[bhavana bajaj [:bajaj]]

NI :wayne to get more information here if this blocking madai as this does not look like a valid bug on our code. Could be a POVB ? We were also not sure of the spec that is mentioned in the description.

Comment 2

[Star Cheng [:scheng]]

In FireFox OS, the "Publicnotification" audio channel type is not supported. In the ConvertChannelToCubebType(), the case dom::AudioChannel::Publicnotification should return CUBEB_STREAM_TYPE_ENFORCED_AUDIBLE to meet the definition of audio stream type of AudioSystem.

static cubeb_stream_type ConvertChannelToCubebType(dom::AudioChannel aChannel)
{
  switch(aChannel) {
    (...)

    // Currently Android openSLES library doesn't support FORCE_AUDIBLE yet.
    case dom::AudioChannel::Publicnotification:
    default:
      NS_ERROR("The value of AudioChannel is invalid");
      return CUBEB_STREAM_TYPE_MAX;
  }
}


So the 'Normal' audio channel is used instead of 'Publicnotification' audio channel. As you saw the log in comment 0:

01-12 19:24:45.959 V/AudioTrack(24964): streamType 1

Comment 3

[Wayne Chang [:wchang]]

marking with [m+] tracker first.

Have spoken to Star, who's now investigating this. Will revisit when some results are available.

Comment 4

[Star Cheng [:scheng]]

Attachment: bug-1007552-fix.patch

support notification audio channel type

Comment 5

[Star Cheng [:scheng]]

(In reply to Seil Park from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101
> Firefox/28.0 (Beta/Release)
> Build ID: 20140314220517
> 
> Steps to reproduce:
> 
> changed camera shutter to 'publicnotification' channel
> 
> Sounds.prototype.createAudio = function(url) {
>   var audio = new Audio(url);
>   // MADAI ONLY. Shutter shound should be always played
>   audio.mozAudioChannelType = 'publicnotification'; // notification
>   return audio;
> };
> 
> I checked stream type in AudioTrack.cpp
> 
> 
> Actual results:
> 
> Currently AudioTrack`s streamType is AUDIO_STREAM_SYSTEM
> 
> 01-12 19:24:45.959 V/AudioTrack(24964): sampleRate 48000, channelMask 0x1,
> format 1
> 01-12 19:24:45.959 V/AudioTrack(24964): streamType 1
> 01-12 19:24:45.959 V/AudioTrack(24964): set() streamType 1 frameCount 0
> flags 0004
> 
> 
> 
> Expected results:
> 
> The shutter sound should be enforced for carrier/country spec. 
> If audio chnnel is 'publicnotification' , it should call
> 'AUDIO_STREAM_ENFORCED_AUDIBLE' stream type. 
> 
> /* Audio stream types */
> typedef enum {
>     AUDIO_STREAM_DEFAULT          = -1,
>     AUDIO_STREAM_VOICE_CALL       = 0,
>     AUDIO_STREAM_SYSTEM           = 1,
>     AUDIO_STREAM_RING             = 2,
>     AUDIO_STREAM_MUSIC            = 3,
>     AUDIO_STREAM_ALARM            = 4,
>     AUDIO_STREAM_NOTIFICATION     = 5,
>     AUDIO_STREAM_BLUETOOTH_SCO    = 6,
>     AUDIO_STREAM_ENFORCED_AUDIBLE = 7, /* Sounds that cannot be muted by
> user and must be routed to speaker */
>     AUDIO_STREAM_DTMF             = 8,
>     AUDIO_STREAM_TTS              = 9,
>     AUDIO_STREAM_INCALL_MUSIC     = 10,
>     AUDIO_STREAM_CNT,
>     AUDIO_STREAM_MAX              = AUDIO_STREAM_CNT - 1,
> } audio_stream_type_t;



Could you try this patch (attachment 8423578 [details] [diff] [review])?  It returns enforced audiable stream type for 'Puliciation' aduio channel type.

Comment 6

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #4)
> Created attachment 8423578 [details] [diff] [review]
> bug-1007552-fix.patch
> 
> 
> support notification audio channel type

In the meantime, I will add publicNotification audio channel for AudioManager in order to controlling volume.

Comment 7

[Star Cheng [:scheng]]

Attachment: bug-1007552-fix.patch

1.support publicnotification audio channel
2.add enforced audiable stream type for opensl

Comment 8

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #5)
> (In reply to Seil Park from comment #0)
> > User Agent: Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101
> > Firefox/28.0 (Beta/Release)
> > Build ID: 20140314220517
> >     AUDIO_STREAM_BLUETOOTH_SCO    = 6,
> >     AUDIO_STREAM_ENFORCED_AUDIBLE = 7, /* Sounds that cannot be muted by
> > user and must be routed to speaker */
> >     AUDIO_STREAM_DTMF             = 8,
> >     AUDIO_STREAM_TTS              = 9,
> >     AUDIO_STREAM_INCALL_MUSIC     = 10,
> >     AUDIO_STREAM_CNT,
> >     AUDIO_STREAM_MAX              = AUDIO_STREAM_CNT - 1,
> > } audio_stream_type_t;
> 
> 
> 
> Could you try this patch (attachment 8423578 [details] [diff] [review])?  It
> returns enforced audiable stream type for 'Puliciation' aduio channel type.

I try to enforced audiable stream type for opensl, and I am not sure it is able to work. Please try this patch (attachment 8423693 [details] [diff] [review]).

Comment 9

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #8)
> (In reply to Star Cheng [:scheng] from comment #5)
> > (In reply to Seil Park from comment #0)
> > > User Agent: Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101
> > > Firefox/28.0 (Beta/Release)
> > > Build ID: 20140314220517
> > >     AUDIO_STREAM_BLUETOOTH_SCO    = 6,
> > >     AUDIO_STREAM_ENFORCED_AUDIBLE = 7, /* Sounds that cannot be muted by
> > > user and must be routed to speaker */
> > >     AUDIO_STREAM_DTMF             = 8,
> > >     AUDIO_STREAM_TTS              = 9,
> > >     AUDIO_STREAM_INCALL_MUSIC     = 10,
> > >     AUDIO_STREAM_CNT,
> > >     AUDIO_STREAM_MAX              = AUDIO_STREAM_CNT - 1,
> > > } audio_stream_type_t;
> > 
> > 
> > 
> > Could you try this patch (attachment 8423578 [details] [diff] [review])?  It
> > returns enforced audiable stream type for 'Puliciation' aduio channel type.
> 
> I try to enforced audiable stream type for opensl, and I am not sure it is
> able to work. Please try this patch (attachment 8423693 [details] [diff] [review]
> [review]).


The SL_ANDROID_STREAM_ENFORCED_AUDIBLE is not supported in opensl ndk lib. 
http://androidxref.com/4.1.1/xref/frameworks/wilhelm/src/android/AudioPlayer_to_android.cpp#464

Comment 10

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #9)
> (In reply to Star Cheng [:scheng] from comment #8)
> > (In reply to Star Cheng [:scheng] from comment #5)
> > > (In reply to Seil Park from comment #0)
> > > > User Agent: Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101
> > > > Firefox/28.0 (Beta/Release)
> > > > Build ID: 20140314220517
> > > >     AUDIO_STREAM_BLUETOOTH_SCO    = 6,
> > > >     AUDIO_STREAM_ENFORCED_AUDIBLE = 7, /* Sounds that cannot be muted by
> > > > user and must be routed to speaker */
> > > >     AUDIO_STREAM_DTMF             = 8,
> > > >     AUDIO_STREAM_TTS              = 9,
> > > >     AUDIO_STREAM_INCALL_MUSIC     = 10,
> > > >     AUDIO_STREAM_CNT,
> > > >     AUDIO_STREAM_MAX              = AUDIO_STREAM_CNT - 1,
> > > > } audio_stream_type_t;
> > > 
> > > 
> > > 
> > > Could you try this patch (attachment 8423578 [details] [diff] [review])?  It
> > > returns enforced audiable stream type for 'Puliciation' aduio channel type.
> > 
> > I try to enforced audiable stream type for opensl, and I am not sure it is
> > able to work. Please try this patch (attachment 8423693 [details] [diff] [review]
> > [review]).
> 
> 
> The SL_ANDROID_STREAM_ENFORCED_AUDIBLE is not supported in opensl ndk lib. 
> http://androidxref.com/4.1.1/xref/frameworks/wilhelm/src/android/
> AudioPlayer_to_android.cpp#464

Hi, Michael

Can we add a new stream type for opensl DNK lib?

Comment 11

[Star Cheng [:scheng]]

> > The SL_ANDROID_STREAM_ENFORCED_AUDIBLE is not supported in opensl ndk lib. 
> > http://androidxref.com/4.1.1/xref/frameworks/wilhelm/src/android/
> > AudioPlayer_to_android.cpp#464
> 
> Hi, Michael
> 
> Can we add a new stream type for opensl DNK lib?

In the audioPlayer_setStreamType() @ frameworks\wilhelm\src\android\ path. Add a new stream type :

    case SL_ANDROID_STREAM_ENFORCED_AUDIBLE:
        newStreamType = AUDIO_STREAM_ENFORCED_AUDIBLE;
        break;

And set the CUBEB_STREAM_TYPE_ENFORCED_AUDIBLE as stream type for testing in AudioStream.cpp

  // fake audiable  stream type
  params.stream_type = CUBEB_STREAM_TYPE_ENFORCED_AUDIBLE;

See the log: we got the stream type "7".

V/AudioPolicyManagerBase(  175): getOutput() stream 7, samplingRate 44100, format 1, channelMask 3, flags 4
V/AudioPolicyManagerBase(  175): getOutput() returns output 2

Comment 12

[Star Cheng [:scheng]]

Attachment: 0001-bug-1007552-add-ENFORCED_AUDIBLE-stream-type-for-cam.patch

add a enforeced audiable stream type for OPENSL ndk lib.

Comment 13

[Star Cheng [:scheng]]

Hi, 

Could you try these 2 patched?  (attachment 8425369 [details] [diff] [review] + Attachment 8423693 [details] [diff])

Comment 14

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #12)
> Created attachment 8425369 [details] [diff] [review]
> 0001-bug-1007552-add-ENFORCED_AUDIBLE-stream-type-for-cam.patch
> 
> 
> add a enforeced audiable stream type for OPENSL ndk lib.

Hi, Michael

We have to add "ENFORCED_AUDIABLE" stream type for opensl ndk lib in order to map "PublicNotification" audio channel type in Gecko. And this modification occurs in AOSP code base. 

Could you tell me how to create a git repository of platform/frameworks/wilhelm (..\frameworks\wilhelm\) in mozilla github b2g?

Comment 15

[Michael Wu [:mwu]]

I can create a fork, but patching system libraries requires coordination with our SoC vendor friends. Otherwise the change will only affect our AOSP based devices (emulator and nexus devices). You should also attempt to push the patch upstream.

Comment 16

[Michael Wu [:mwu]]

https://github.com/mozilla-b2g/platform_frameworks_wilhelm created.

What android base versions do you want to support this on?

Comment 17

[Star Cheng [:scheng]]

(In reply to Michael Wu [:mwu] from comment #15)
> I can create a fork, but patching system libraries requires coordination
> with our SoC vendor friends. Otherwise the change will only affect our AOSP
> based devices (emulator and nexus devices). You should also attempt to push
> the patch upstream.

Thank you help to create the fork and remind these things.

Comment 18

[Star Cheng [:scheng]]

(In reply to Michael Wu [:mwu] from comment #16)
> https://github.com/mozilla-b2g/platform_frameworks_wilhelm created.
> 
> What android base versions do you want to support this on?


This feature (provide publicnotification audio channel for camera shutter) is for Madai, and it's gonk is Android KK. So I want to support it on android-4.4.2_r1. Do you have any suggestion?

Comment 19

[Star Cheng [:scheng]]

I will provide patch for this bug, and remove the WIP patch and ni? tag.

Comment 20

[Star Cheng [:scheng]]

Attachment: bug-1007552-fix.patch

Add fourced audiable stream type in gecko.

Comment 21

[Seil Park]

I`m sorry for the delay in my response. I`m following up another project. 
I checked these patches and It is fine. 
https://bug1007552.bugzilla.mozilla.org/attachment.cgi?id=8425369
https://bug1007552.bugzilla.mozilla.org/attachment.cgi?id=8426842
After releasing these patchese, I will review shutter sound scenario with gaia member.
thanks.

Comment 22

[Matthew Gregan [:kinetik]]

Comment on attachment 8426842 [details] [diff] [review]
bug-1007552-fix.patch

Happy to take the patch with the issues I mentioned addressed.

Comment 23

[Star Cheng [:scheng]]

(In reply to Matthew Gregan [:kinetik] from comment #22)
> Comment on attachment 8426842 [details] [diff] [review]
> bug-1007552-fix.patch
> 
> Happy to take the patch with the issues I mentioned addressed.


Hi,  Matthew


Thank your review. And do you mind to tell me what is your concern and what I have to modify. I will try to fix it.

Comment 24

[Matthew Gregan [:kinetik]]

Sorry, I wrote comments in the review UI and they seem to have disappeared. :-(

From memory:
- rename ENFORCED_AUDIBLE to SYSTEM_ENFORCED as this is the name used in AudioManager.java.
- remove the comment line about KitKat

Thanks!

Comment 25

[Star Cheng [:scheng]]

Attachment: bug-1007552-fix.patch

1.rename ENFORCED_AUDIBLE to SYSTEM_ENFORCED in sles_definitions.h as this is the name used in AudioManager.java.
2.remove the comment line about KitKat in cubeb_opensl.c

Comment 26

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #18)
> (In reply to Michael Wu [:mwu] from comment #16)
> > https://github.com/mozilla-b2g/platform_frameworks_wilhelm created.
> > 
> > What android base versions do you want to support this on?
> 
> 
> This feature (provide publicnotification audio channel for camera shutter)
> is for Madai, and it's gonk is Android KK. So I want to support it on
> android-4.4.2_r1. Do you have any suggestion?


Hi, Michael

I have pushed these modification in repository of my gitHub [1].  But I can't pull request to Tag (android-4.4.2_r1) of Mozilla git hub. Could you help to create a branch of android-4.4.2_r1 for upstream the modification of opensl lib? 


[1] https://github.com/littlestar0425/platform_frameworks_wilhelm/commit/e1c695dd3184dd87ddbdb4a72fb6996aab2d820c

Comment 27

[Matthew Gregan [:kinetik]]

Comment on attachment 8430497 [details] [diff] [review]
bug-1007552-fix.patch

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

r+ with comments addressed

::: content/media/AudioStream.cpp
@@ +136,2 @@
>      case dom::AudioChannel::Publicnotification:
> +      return CUBEB_STREAM_TYPE_ENFORCED_AUDIBLE;

Can you please rename the cubeb constant too?  The names were originally taken from Android, so I'd like to keep them in sync.

Comment 28

[Michael Wu [:mwu]]

(In reply to Star Cheng [:scheng] from comment #26)
> Could you
> help to create a branch of android-4.4.2_r1 for upstream the modification of
> opensl lib? 
>

Done. b2g-4.4.2_r1 generated.

Comment 29

[Star Cheng [:scheng]]

Attachment: B2G GitHub pull request

1.To add SYSTEM_ENFORCED type for OpenSL ndk lib in wilhelm. 
2.This is only for whilhelm in 4.4..2_r1 branch of AOSP.

Comment 30

[Matthew Gregan [:kinetik]]

Oh, so it *is* called ENFORCED_AUDIBLE some places inside the Android code:

+        newStreamType = AUDIO_STREAM_ENFORCED_AUDIBLE;

Confusing. :-/

This looks fine to be, but not sure if I can r+ mozilla-b2g stuff.

Comment 31

[Star Cheng [:scheng]]

(In reply to Matthew Gregan [:kinetik] from comment #30)
> Oh, so it *is* called ENFORCED_AUDIBLE some places inside the Android code:
> 
> +        newStreamType = AUDIO_STREAM_ENFORCED_AUDIBLE;
> 
> Confusing. :-/
> 
> This looks fine to be, but not sure if I can r+ mozilla-b2g stuff.

Oh yeh! AudioManager use STREAM_SYSTEM_ENFORCED, and AUDIO_STREAM_ENFORCED_AUDIBLE is used in Audio.h instead :(


[1] http://androidxref.com/4.4.2_r2/xref/frameworks/base/media/java/android/media/AudioManager.java#216
[2] http://androidxref.com/4.4.2_r2/xref/system/core/include/system/audio.h#49

Comment 32

[Star Cheng [:scheng]]

Attachment: bug-1007552-fix-v2.patch

1.TPBL result : https://tbpl.mozilla.org/?tree=Try&rev=17d1a96652a2
2.add reviewer

Comment 33

[Star Cheng [:scheng]]

There are 2 patches for this issue. One is for Gecko, the other is for Mozilla B2G GitHub

1. attachment 8433874 [details] [diff] [review] : This patch is for Gecko
2. Attachment 8432969 [details] : This patch is for B2G GitHub

Comment 34

[Star Cheng [:scheng]]

(In reply to Star Cheng [:scheng] from comment #32)
> Created attachment 8433874 [details] [diff] [review]
> bug-1007552-fix-v2.patch
> 
> 
> 1.TPBL result : https://tbpl.mozilla.org/?tree=Try&rev=17d1a96652a2
> 2.add reviewer

3. rename ENFORCED_AUDIBLE to SYSTEM_ENFORCED for cubeb constant

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/12f6d73e8fb1

b2g-4.4.2_r1: https://github.com/mozilla-b2g/platform_frameworks_wilhelm/commit/98289897bc4b0035d6ec799f03e838ac3c9052cd

In the future, please try to be more conscientious about what platforms and test suites you use on your Try pushes. "All" runs consume over 300hr of machine time and contribute to long backlogs for other developers. Thanks!
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/12f6d73e8fb1

Comment 37

[Star Cheng [:scheng]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #35)
> https://hg.mozilla.org/integration/b2g-inbound/rev/12f6d73e8fb1
> 
> b2g-4.4.2_r1:
> https://github.com/mozilla-b2g/platform_frameworks_wilhelm/commit/
> 98289897bc4b0035d6ec799f03e838ac3c9052cd
> 
> In the future, please try to be more conscientious about what platforms and
> test suites you use on your Try pushes. "All" runs consume over 300hr of
> machine time and contribute to long backlogs for other developers. Thanks!
> https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Hi, Ryan

Thank you remind me about that. I will considerate of our try server resource.

Comment 38

[Michael Wu [:mwu]]

This bug isn't actually fixed until patches reach SoC vendor trees (CAF and spreadtrum). Upstreaming to AOSP will help with that. Please let me know the progress on this.