858469 - [Camera][Sound] occur abnormal Video Recording start sound

Status: RESOLVED FIXED

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

Description

[leo.bugzilla.gecko]

[Precondition] 
 Use headphone - Try to record video
[Symptom] 
 abnormal starting/ending sound by headphones or speaker. 
[Frequency Rate] : 90%
[Request]
 need to delay starting/ending sound from recording.

[Reason]
 If it uses headphone, starting/ending sound should route speaker and headphone at the same time.
 At this moment, It happens delay time of routing in Audio library.
 Can you adjust starting/ending time`s tunning?

Comment 1

[Andrew Overholt [:overholt]]

Leo, could you please provide a bit more information about what the user impact is?

Sotaro, do you have any thoughts here?

Comment 2

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

I can reproduce this, and it's a little weird.

STR:
0. plug in headphones and put one in your ear/uncover one ear
1. open camera app
2. switch to video mode
3. press start recording
--- observe: the *bong* sound only comes from the phone's speaker, not the headphones
4. press stop recording
--- observe: the *bong* sound initially only comes from the phone's speaker, but about halfway through, the headphones turn on and the sound comes from them as well (you can tell by the sudden-but-gentle *pop* in the headphones and the presence of white noise in the background)

Observed on:
- unagi
- gecko: inbound-src:45835d0c9d58 / non-debug, opt, no-dmd
- gaia: 2f64311e0f23b85c29b20be9502cbdeaee8342ae

Opinion: annoying paper cut.

Comment 3

[Sotaro Ikeda [:sotaro]]

I took a look. Recording start/stop sound is itself incorrect sound... We need to stop it. It is not played by gaia app. It is played in android's CameraService in media server process.

A shutter sound by CameraService is already stopped by Bug 851823. But CameraService do not provide an api for stopping recording start/stop sound. A simple way to stop the sound is remove "/system/media/audio/ui/VideoRecord.ogg" from ROM. IIRC, until recently, the file is not included in the ROM, I think. When I was fixing Bug 803471, ROM did not include the file.

Comment 4

[Sotaro Ikeda [:sotaro]]

I assume the wired sound comes from audio playback without taking cpu WakeLock. In android AudioFlinger took cpu wake lock when necessary. In Firefox OS, there is no android's PowerManager, the AudioFlinger can not hold cpu WakeLock. In FirefoxOS, app needs to take cpu WakeLock during audio playback.

Comment 5

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> I took a look. Recording start/stop sound is itself incorrect sound... We
> need to stop it. It is not played by gaia app. It is played in android's
> CameraService in media server process.
> 

When I remove the recording start/stop sound by changing CameraService's source code. The sound stop. I confirmed on unagi.

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: patch - remove android's sound files

Comment 7

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> Created attachment 733962 [details] [diff] [review]
> patch - remove android's sound files

The android's sound files are not necessary for Forefox OS. They can be removed from ROM.

Comment 8

[Sotaro Ikeda [:sotaro]]

Comment on attachment 733962 [details] [diff] [review]
patch - remove android's sound files

mwu, can you review the patch? I confirmed the patch on unagi.

Comment 9

[Joe Cheng [:jcheng] (please needinfo)]

ni? on reporter to understand more on the user impact and bug description

Comment 10

[Michael Wu [:mwu]]

mvines, you'll probably want to port this patch to your platform/build repo.

Comment 11

[Michael Vines [:m1] [:evilmachines]]

(In reply to Michael Wu [:mwu] from comment #10)
> mvines, you'll probably want to port this patch to your platform/build repo.

Thanks for the heads up.

Comment 12

[Sotaro Ikeda [:sotaro]]

I recognized that the bug was already fixed at https://github.com/mozilla-b2g/platform_build by Bug 825142.

Comment 13

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> I recognized that the bug was already fixed at
> https://github.com/mozilla-b2g/platform_build by Bug 825142.

mwu, the above change does not appear on v1-train branch.

Comment 14

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> I recognized that the bug was already fixed at
> https://github.com/mozilla-b2g/platform_build by Bug 825142.

The change is not uplifted yet. It needs leo+.

Comment 15

[Sotaro Ikeda [:sotaro]]

mwu, do you know how to uplift to v1-train branch? The bug is already leo+.

Comment 16

[Sotaro Ikeda [:sotaro]]

Attachment: link to patch on github

Comment 17

[Sotaro Ikeda [:sotaro]]

Comment on attachment 733962 [details] [diff] [review]
patch - remove android's sound files

obsolete the patch. There was already same change applied in master. Just uplift it to v1-traing is enough.

Comment 18

[Sotaro Ikeda [:sotaro]]

need to update also b2g-manifest of v1-train
https://github.com/mozilla-b2g/b2g-manifest/tree/v1-train

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Updated b2g-manifest's v1-train branch with a1cdaefcc1f91d7878723a9d906ba513dc3f8c78

Comment 20

[Sotaro Ikeda [:sotaro]]

I confirmed the fix on unagi.

Comment 21

[Sotaro Ikeda [:sotaro]]

need to update also b2g-manifest of v1-train and v1.0.1

Comment 22

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #21)
> need to update also b2g-manifest of v1-train and v1.0.1

Sorry, it was a comment for Bug 862097. Ignore it.

Comment 23

[leo.bugzilla.gecko]

Dear Sotaro Ikeda [:sotaro] 

Can you test headset sound after pluging headset?
I reprodueced this issue(cutting sound by headset).

https://github.com/mozilla-b2g/platform_build/commit/68be583924113b6840f19b2654e879916db0a495
-------------------------------------------------------------------------------
target/product/full_base.mk
# $(call inherit-product-if-exists, frameworks/base/data/sounds/AllAudio.mk)
-------------------------------------------------------------------------------

Comment 24

[leo.bugzilla.gecko]

This patch means that start/stop recording sound file is only removed?
If you are right, please change fixed status. 

Leo model is currently using the other sound path of model. So I can hear recording sound.
We will remove recording sound files. thanks.

Comment 25

[Sotaro Ikeda [:sotaro]]

(In reply to leo.bugzilla.gecko from comment #24)
> This patch means that start/stop recording sound file is only removed?

Yes. 

> If you are right, please change fixed status. 
> 
> Leo model is currently using the other sound path of model. So I can hear
> recording sound.
> We will remove recording sound files. thanks.

Thanks for confirmation.

Comment 26

[Michael Wu [:mwu]]

mvines, is this patch available on your side? I checked the patches and b2g/* branches and didn't see anything.

Comment 27

[Michael Vines [:m1] [:evilmachines]]

I think the patch is in the frameworks/base area but not sure off hand, these audio files aren't in the build though