Closed
Bug 851823
Opened 10 years ago
Closed 10 years ago
Too much Camera shutter being available (Nexus S only ?)
Categories
(Firefox OS Graveyard :: Gaia::Camera, defect)
Tracking
(firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)
RESOLVED
FIXED
B2G C4 (2jan on)
People
(Reporter: gerard-majax, Assigned: ben.tian)
References
Details
(Whiteboard: [TD-8411])
Attachments
(1 file, 1 obsolete file)
946 bytes,
patch
|
sotaro
:
review+
akeybl
:
approval-mozilla-b2g18+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•10 years ago
|
||
A quick fix for this: disabling the camera shutter from CameraService.
Assignee: nobody → lissyx+mozillians
Status: NEW → ASSIGNED
Comment 2•10 years ago
|
||
Alexandre, could you please add a simple STR so that we could try on Unagi and possibly ask for tracking ?
Reporter | ||
Comment 3•10 years ago
|
||
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•10 years ago
|
||
The file does not exist on my Unagi.
Comment 5•10 years ago
|
||
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•10 years ago
|
||
(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".
Reporter | ||
Comment 7•10 years ago
|
||
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•10 years ago
|
||
It is same as Bug 853552.
Comment 10•10 years ago
|
||
a patch from Bug 853552.
Attachment #725791 -
Attachment is obsolete: true
Attachment #728210 -
Flags: review+
Comment 11•10 years ago
|
||
:gerard-majax, I reviewed a patch at Bug 853552. The patch is a good patch to fix the problem.
Comment 12•10 years ago
|
||
:bentian, can you handle the patch in the bug? The patch is already "review+".
Assignee | ||
Comment 13•10 years ago
|
||
: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?
Reporter | ||
Comment 14•10 years ago
|
||
Of course not, its the same patch :)
Assignee | ||
Updated•10 years ago
|
Assignee: lissyx+mozillians → btian
Assignee | ||
Comment 15•10 years ago
|
||
(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.
Keywords: checkin-needed
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e6a306cbc51
Keywords: checkin-needed
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9e6a306cbc51
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
Comment 18•10 years ago
|
||
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 ?
Updated•10 years ago
|
Flags: needinfo?(btian)
Comment 19•10 years ago
|
||
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.
Flags: needinfo?(btian)
Comment 20•10 years ago
|
||
Oh fine, I understand now. I'm requesting approval then.
Comment 21•10 years ago
|
||
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
Attachment #728210 -
Flags: approval-mozilla-b2g18?
Updated•10 years ago
|
Attachment #728210 -
Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Comment 22•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/1970ba604a08
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox20:
--- → wontfix
status-firefox21:
--- → wontfix
status-firefox22:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•