851823 - Too much Camera shutter being available (Nexus S only ?)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect)

Description

[Alexandre LISSY :gerard-majax]

After updating my Gaia and Gecko today, I noticed a camera shutter sound when taking pictures, while in settings it was explicitely disabled.

After digging a bit, I found that it's an Android camera shutter in action. More precisely, renaming /system/media/audio/ui/camera_click.ogg to /system/media/audio/ui/camera_click.bak makes the sound going away. After digging a bit more, one can read the source code of libcameraservice, in frameworks/base/services/camera/libcameraservice, and especially the CameraService.cpp file. As far as I can understand, it seems the gecko side of this layer is in netwerk/protocol/device/gonk/Camera.h.

Shutter sound is configured by:

void CameraService::loadSound() {
    Mutex::Autolock lock(mSoundLock);
    LOG1("CameraService::loadSound ref=%d", mSoundRef);
    if (mSoundRef++) return;

    mSoundPlayer[SOUND_SHUTTER] = newMediaPlayer("/system/media/audio/ui/camera_click.ogg");
    mSoundPlayer[SOUND_RECORDING] = newMediaPlayer("/system/media/audio/ui/VideoRecord.ogg");
}

Playing (or not) the shutter is controlled by the local variable mPlayShutterSound, whose access is encapsulated by status_t CameraService::Client::enableShutterSound(bool enable), as one can see in the handleShutter method:

// snapshot taken callback
void CameraService::Client::handleShutter(void) {
    if (mPlayShutterSound) {
        mCameraService->playSound(SOUND_SHUTTER);
    }

    sp<ICameraClient> c = mCameraClient;
    if (c != 0) {
        mLock.unlock();
        c->notifyCallback(CAMERA_MSG_SHUTTER, 0, 0);
        if (!lockIfMessageWanted(CAMERA_MSG_SHUTTER)) return;
    }
    disableMsgType(CAMERA_MSG_SHUTTER);

    mLock.unlock();
}

Looking at the Camera.h file of gecko, we see that there is no enableShutterSound glue, thus no way for Gecko, and Gaia/Camera application to disable the shutter at the Android level.

Comment 1

[Alexandre LISSY :gerard-majax]

Attachment: Disabling Android-level Camera shutter sound

A quick fix for this: disabling the camera shutter from CameraService.

Comment 2

[Julien Wajsberg [:julienw]]

Alexandre, could you please add a simple STR so that we could try on Unagi and possibly ask for tracking ?

Comment 3

[Alexandre LISSY :gerard-majax]

Sure.

Steps to reproduce are:

0. Ensure that the file /system/media/audio/ui/camera_click.ogg exists
1. Take a picture with Camera application and shutter sound disabled in settings

Expected:
No sound

Current behavior:
The /system/media/audio/ui/camera_click.ogg sound gets played.

Comment 4

[Julien Wajsberg [:julienw]]

The file does not exist on my Unagi.

Comment 5

[Mike Habicher [:mikeh] (high bugzilla latency)]

Comment on attachment 725791 [details] [diff] [review]
Disabling Android-level Camera shutter sound

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

::: dom/camera/GonkCameraHwMgr.cpp
@@ +180,5 @@
> +  /**
> +    * From system/core/include/system/camera.h: CAMERA_CMD_ENABLE_SHUTTER_SOUND = 4
> +    * Disables Android-level Camera shutter sound.
> +    **/
> +  mCamera->sendCommand(4, 0, 0);

Is this a known/standard command?  Or is it Nexus S-specific?  If the latter, will there be unknown side-effects on other platforms?

Comment 6

[Sotaro Ikeda [:sotaro]]

(In reply to Mike Habicher [:mikeh] from comment #5)
> Comment on attachment 725791 [details] [diff] [review]
> Disabling Android-level Camera shutter sound
> 
> Review of attachment 725791 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/camera/GonkCameraHwMgr.cpp
> @@ +180,5 @@
> > +  /**
> > +    * From system/core/include/system/camera.h: CAMERA_CMD_ENABLE_SHUTTER_SOUND = 4
> > +    * Disables Android-level Camera shutter sound.
> > +    **/
> > +  mCamera->sendCommand(4, 0, 0);
> 
> Is this a known/standard command?  Or is it Nexus S-specific?  If the
> latter, will there be unknown side-effects on other platforms?

It  is a known/standard command. It is present in normal ICS and JB source code.

It seem that "CAMERA_CMD_ENABLE_SHUTTER_SOUND" should be used instead of "4". "system/camera.h" is included in "/frameworks/base/include/camera/Camera.h". It is already included at "GonkCameraHwMgr.h".

Comment 7

[Alexandre LISSY :gerard-majax]

Thanks, since I did this as a quick workaround, I did not took the chance to get into troubles of headers :). If this is confirmed to be needed, I'll make a nicer patch.

Comment 8

[Sotaro Ikeda [:sotaro]]

It is same as Bug 853552.

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: Disable shutter sound in CameraService

a patch from Bug 853552.

Comment 11

[Sotaro Ikeda [:sotaro]]

:gerard-majax, I reviewed a patch at Bug 853552. The patch is a good patch to fix the problem.

Comment 12

[Sotaro Ikeda [:sotaro]]

:bentian, can you handle the patch in the bug? The patch is already "review+".

Comment 13

[Ben Tian (inactive)]

:sotaro, no problem. I will handle the patch.
https://tbpl.mozilla.org/?tree=Try&rev=ecad55960d66

:gerard-majax, do you mind my taking this bug?

Comment 14

[Alexandre LISSY :gerard-majax]

Of course not, its the same patch :)

Comment 15

[Ben Tian (inactive)]

(In reply to Ben Tian [:bentian] from comment #13)
> https://tbpl.mozilla.org/?tree=Try&rev=ecad55960d66

The oranges on platforms other than B2G are unrelated to my patch since Camera API is enabled only on B2G. The remaining B2G oranges [3] and [R3] are unrelated failures for all B2G builds nearby encounter [3] failure and [R3] fails before CameraService started.

Comment 16

[[Back July 11] Ryan VanderMeulen]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e6a306cbc51

Comment 17

[[Back July 11] Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/9e6a306cbc51

Comment 18

[Julien Wajsberg [:julienw]]

Ben, I don't know this code well but I don't see where we reenable the SHUTTER_SOUND when the user turns it on again ?

Comment 19

[Dale Harvey (:daleharvey)]

We dont, this disables the hardware shutter entirely as the hardware shutter doesnt exist in the unagi and gaia is responsible for playing the shutter in the case it is enabled.

Comment 20

[Julien Wajsberg [:julienw]]

Oh fine, I understand now. I'm requesting approval then.

Comment 21

[Julien Wajsberg [:julienw]]

Comment on attachment 728210 [details] [diff] [review]
Disable shutter sound in CameraService

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: the shutter sound will be heard on some phones even if the user disabled it
Testing completed: yes
Risk to taking this patch (and alternatives if risky): very low, one-liner, and this probably has no impact on our target phone
String or UUID changes made by this patch: none

Comment 22

[[Back July 11] Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/1970ba604a08