[B2G][Helix][Camera]There is no tone when start or stop a video recording

RESOLVED FIXED

Status

P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ysdingyu2013, Assigned: johnhu)

Tracking

unspecified

Firefox Tracking Flags

(blocking-b2g:-)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; Tablet PC 2.0)

Steps to reproduce:

1.Open the camer app
2.start and stop a video recording


Actual results:

2.there is no tone when start or stop a video recording


Expected results:

2.It should  have a tone when start or stop a video recording
(Reporter)

Updated

5 years ago
Priority: -- → P1
Hi Sri,
This may be required by law in some regions. Have we got this planned or backlogged somewhere?

Hi Dale, 
Can you estimate the LOE for this? to have audible indication on start and stop of video recording?
Flags: needinfo?(skasetti)
Flags: needinfo?(dale)
I havent seen it in any backlog, it will likely require a new setting for locations where it isnt a legal requirement but I would expect it to be a fairly small amount of work given the assets
Flags: needinfo?(dale)
Thanks Dale. Let me flag product here for some input.

(In reply to Dale Harvey (:daleharvey) from comment #2)
> I havent seen it in any backlog, it will likely require a new setting for
> locations where it isnt a legal requirement but I would expect it to be a
> fairly small amount of work given the assets
Flags: needinfo?(ffos-product)

Comment 4

5 years ago
DingYu, Wayne, I just tested this feature on the latest V1.1 build.
When you touch the video icon to start the recording, you can hear a tone and then the icon turns red.When you stop the recording, you can hear the same tone. Am I missing something here?
Flags: needinfo?(skasetti)
Flags: needinfo?(ffos-product)
FYI, CameraService's recoring start sound is disabled in Bug 858469.
(Reporter)

Comment 6

5 years ago
Sri Kasetti, It's just I want. By the way, I have a question, In setting-->sound-->Camera shutter, the option can enable and disable the camera shutter tone, The video tone is controled by this option too?
Sri, it seems that it'd been removed, and also on other devices there is no sound. What build are you running?

Adding William here who confirmed the sound missing.

Sotaro, 858469 seemed to have removed the sound to eliminate some unwanted behaviour. Can that be fixed correctly so that we can add sound indication if required?

Updated

5 years ago
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(skasetti)
(In reply to DingYu from comment #6)
> Sri Kasetti, It's just I want. By the way, I have a question, In
> setting-->sound-->Camera shutter, the option can enable and disable the
> camera shutter tone, The video tone is controled by this option too?

Current Firefox OS does not implement the video recording tone.
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Wayne Chang [:wchang] from comment #7)
> 
> Sotaro, 858469 seemed to have removed the sound to eliminate some unwanted
> behaviour. Can that be fixed correctly so that we can add sound indication
> if required?

The bug just remove video recording tone file from ROM. It works when the build is same as AOSP. If partner built ROM modified it, the fix does not work. In fact, Leo changed how the build script extracts the recording tone. And fixed it on leo side by different ways.
Hema, would you triage this for backlogging as well?
Flags: needinfo?(hkoka)

Comment 11

5 years ago
Will add this to the backlog list

Thanks
Hema
Flags: needinfo?(hkoka)

Comment 12

5 years ago
Responding to comment 7 from Wayne. Looks like my build was an earlier one.
Satoro updated my build. So I don't get the tone now.
As Hema said we will add this to our backlog.
Flags: needinfo?(skasetti)
blocking-b2g: --- → leo?

Comment 13

5 years ago
Just flagging UX in case we want to give input here. If it's fine as-is, that's OK.
Flags: needinfo?(jsavory)
moving to media team for 1.2 backlog. Made a koi?
blocking-b2g: leo? → koi?
Not a regression, and no partner/certification requirements around this yet so not blocking yet.

Once it's clear which markets legally require it, re-nominate.
blocking-b2g: koi? → -
Flags: needinfo?

Comment 16

5 years ago
As discussed in the sprint planning, we will be taking this on. 

John, assigning this to you
Assignee: nobody → johu
Flags: needinfo?
Yes, no problem. I can start to work on this.
Because waiting for the suggested sound effect, I will discuss this one with system platform team to add the support of recording tone as a build-time configuration. So, the OEM can customize it and we can also use the same behavior to add the default sound effect.
I found the enablement of shutter sound of camera was already in build-time customization list. I will use the same setting for the enablement of the recording sound and add an additional filename for recording sound which is not customizable.

Because we are waiting for sound effect from UX, I will copy the original camera shutter sound as the recording sound. And open a bug for the replacement of sound effect, if the code is landed before sound effect ready.
Created attachment 798772 [details]
extract sound effect object and add shutter sound to start/stop recording

1. extract the code about shutter sounds to SoundEffect object.
2. add shutter sound effect to the place starting/stoping recording.
3. copy shutter.ogg as camcorder.ogg. I will open a follow-up bug for sound effect replacement, if this patch is landed before UX gives us sound effect.
Attachment #798772 - Flags: review?(dflanagan)
Sri, 

How do you think about the customization of sound effect resources?? I means OEM can change the sound effects, shutter.ogg and camcorder.ogg, at the build-time.
Flags: needinfo?(skasetti)
Comment on attachment 798772 [details]
extract sound effect object and add shutter sound to start/stop recording

John,

I have a number of comments on github. My primary concerns with this patch are:

1) I think you may be playing the sounds too late.

2) It would be more efficient in soundeffect.js to not create the Audio elements if the sounds are turned off in settings.

3) I'd guess that UX will want different sounds for "start recording" and "stop recording" so you may have to modify your code for that.
Attachment #798772 - Flags: review?(dflanagan) → review-

Comment 23

5 years ago
(In reply to John Hu [:johnhu] from comment #21)
> Sri, 
> 
> How do you think about the customization of sound effect resources?? I means
> OEM can change the sound effects, shutter.ogg and camcorder.ogg, at the
> build-time.

We don't need to make this build time customizable
Flags: needinfo?(skasetti)

Updated

5 years ago
Flags: needinfo?(rmacdonald)
Comment on attachment 798772 [details]
extract sound effect object and add shutter sound to start/stop recording

The followings are changed in this patch:
1. move the sound playing before recording start and right after recording end.
2. create the audio element when needed and use cloneNode to create a cloned one for playing rapidly.
3. add recording start and recording end API to have different start and end sound resources. 
4. rename the variables.
Attachment #798772 - Flags: review- → review?(dflanagan)
Comment on attachment 798772 [details]
extract sound effect object and add shutter sound to start/stop recording

John,

You've addressed all my code concerns. Thank you!  I haven't had time to try the patch myself, but if you have tested it, the go ahead and land.

Please do ensure that none of the recording end sound is captured in the recorded video.  And check that there isn't a very noticeable delay for the very first picture while the shutter sound is loaded.
Attachment #798772 - Flags: review?(dflanagan) → review+
Hi John...

Thanks for flagging me on this. Would it be possible for you to package the attachment as per James Burke's instructions? 

https://github.com/jrburke/gaia-dev-zip#for-the-developer 

This will allow UX and PM to review the attachment. 

Otherwise, David is correct in his assumption that we would want a different sound at the start and the end of the recording. It's really important to differentiate between the two.
Flags: needinfo?(rmacdonald)
Flags: needinfo?(jsavory)
(In reply to Rob MacDonald [:robmac] from comment #26)
> Hi John...
> 
> Thanks for flagging me on this. Would it be possible for you to package the
> attachment as per James Burke's instructions? 
> 
> https://github.com/jrburke/gaia-dev-zip#for-the-developer 

I will do it right away. But this bug still need the sound resources. I thought the need info flag is to request those sound resources. That's why I didn't put ui-review on you. But I will do it now.


(In reply to David Flanagan [:djf] from comment #25)
> Comment on attachment 798772 [details]
> Please do ensure that none of the recording end sound is captured in the
> recorded video.  And check that there isn't a very noticeable delay for the
> very first picture while the shutter sound is loaded.

Hi David, 

I had confirmed that before reseting the review. The start and end sound didn't recorded into video. And as to the delay, I can't notice it with my low-end device.
Created attachment 800531 [details]
bug896874-recording-tone.zip

Hi Rob and Sri,

The attachment is the patched version. And I just use the camera shutter sound should to be the recording start and recording end sound.

Thanks...
Attachment #800531 - Flags: ui-review?(rmacdonald)
Attachment #800531 - Flags: review?(skasetti)
Created attachment 801341 [details]
bug896874-recording-sound-2.zip

use the notify_bap for recording start and notify_bop for recording stop.
Attachment #800531 - Attachment is obsolete: true
Attachment #800531 - Flags: ui-review?(rmacdonald)
Attachment #800531 - Flags: review?(skasetti)
Attachment #801341 - Flags: ui-review?(rmacdonald)
According to the email discussion with Patryk, I will add a setting item into Settings app, under Sound->Other sounds->Recording Sound (new one).
Comment on attachment 798772 [details]
extract sound effect object and add shutter sound to start/stop recording

The new patch only add a new setting item and use it to determine if recording sound is played or not, about 90% of camera code remaining the same. So, I didn't reset the review+ flag of David.

I add another review flag to Arthur for reviewing the new setting item to settings app.
Attachment #798772 - Flags: review?(arthur.chen)
Created attachment 802002 [details]
video for reviewing

Rob, 

To help the reviewing, I make a video of the patch. I already added a new setting item to settings app. And the tool of UX-review can't pack two apps. So, I make this video to show the result of changing settings.

The attachment 801341 [details] is the version that always enabled recording sound. So, you can test it without the update of settings app and the build.
Attachment #802002 - Attachment description: video → video for reviewing
Comment on attachment 798772 [details]
extract sound effect object and add shutter sound to start/stop recording

Simple change. r=me.
Attachment #798772 - Flags: review?(arthur.chen) → review+
Comment on attachment 801341 [details]
bug896874-recording-sound-2.zip

Minused based on wording of "Settings". Please change "Video recording sound" to "Video recording" to make it consistent with the other settings. 

Otherwise everything seems fine from an interaction perspective. I initially felt there was a bit of lag at the start of the recording but this is a separate issue from this bug.

Also I will flag Patryk for review of the audio file and Carrie for a review of the Settings. 

Thanks also for putting together the zip file and video - extremely helpful! :)
Attachment #801341 - Flags: ui-review?(rmacdonald) → ui-review-
Flags: needinfo?(padamczyk)
Flags: needinfo?(cawang)
The audio files sound correct to me.
Flags: needinfo?(padamczyk)
Created attachment 802736 [details]
updated settings UI
Rob, Patryk, and Carrie, 

Thanks for your information. I had changed the patch to have "Video recording". Please see the attachment 802736 [details].

As to the following question, I also think this is another issue, but I can explain something here.

> Otherwise everything seems fine from an interaction perspective. I initially
> felt there was a bit of lag at the start of the recording but this is a
> separate issue from this bug.

The root of this lag is that we need to do: 1. create a filename according to design rule of camera file system, 2. create a dummy file for writing test, and 3. check the free space for it. The total consumes about 300ms. That may be improved. I had tested different places to play the sound. Like, I can play it after user pressed the button, before video recording and after video recording. But I prefer to play it right before video recording because it reflects the real status.

You may notice another lag of the enablement of capture button, that's on purpose. We met the bug 899864 before. Finally, we found the recorded video files lack audio/video samples because user press capture button too fast. So, we delayed the enablement of capture button when recording. Android plays the same trick, too.
Comment on attachment 801341 [details]
bug896874-recording-sound-2.zip

I had changed the wording of settings item. So, I put the ui-review? back.
Attachment #801341 - Flags: ui-review- → ui-review?(rmacdonald)
I'm fine with the new string. Thanks.
Flags: needinfo?(cawang)
Comment on attachment 801341 [details]
bug896874-recording-sound-2.zip

(In reply to John Hu [:johnhu] from comment #37)
> Rob, Patryk, and Carrie, 
> 
> Thanks for your information. I had changed the patch to have "Video
> recording". Please see the attachment 802736 [details].

Hi John...

So close! :) The zip file looks great. However, in the screenshot (attachment 802736 [details]) the string "Camera recording" needs to be changed to "Video recording" as per your comment above. 

No need to add me for another review if you modify the string. And thanks also for the useful background information.

Best regards,
Rob
Attachment #801341 - Flags: ui-review?(rmacdonald) → ui-review-
Created attachment 803443 [details]
updated settings UI - 2

Oh, I am so sorry about this. I just forgot to replace the "Camera" as "Video". I just removed the word "sound".

This attachment has the correct words.
Attachment #802736 - Attachment is obsolete: true
Comment on attachment 801341 [details]
bug896874-recording-sound-2.zip

As per rob's comment, comment 40, I removed the r- flag from this file.
Attachment #801341 - Flags: ui-review-
mreged to master:

https://github.com/mozilla-b2g/gaia/commit/d2f2ea5e3f402cba11ace91c35449be62d4ce53c
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 44

5 years ago
Hi Wayne :
 Please help us to push to merge this case into V1.1 HD. It blocked the Helix Device .If you can not resolve it , please get the authorization from carrier.

Thanks.

Updated

5 years ago
Flags: needinfo?(wchang)
We're not taking changes on v1.1.

This has however been included on v1.2 branch.
Flags: needinfo?(wchang)
You need to log in before you can comment on or make changes to this bug.