Inder Reporter
	Description • 12 years ago

Steps to reproduce:
1. Start playing an audio clip
2. Goto camera and switch to camcorder
3. Start recording
4. On starting the recording, camcorder crashes with camera just crashed message

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 1 • 12 years ago

basecamp+ assuming that the user is going to need to figure out that they have to kill the audio clip before they can use the camera. ie. When the camera crashes it will continue to crash until the user manually stops the audio stream.

Note that we need a public version of this bug to complete the work.

	Doug Turner (:dougt)
	Comment 2 • 12 years ago

Mike, can you take a look?

	Mike Habicher [:mikeh] (high bugzilla latency)
	Comment 3 • 12 years ago

(In reply to Lawrence Mandel [:lmandel] from comment #1)
> basecamp+ assuming that the user is going to need to figure out that they
> have to kill the audio clip before they can use the camera. ie. When the
> camera crashes it will continue to crash until the user manually stops the
> audio stream.

I don't think the user should have to do this manually.  When I open the camera on my BlackBerry, it automatically suspends any playing audio.  (I assume this is because the DSP can't support both operations at the same time.)

(In reply to Doug Turner (:dougt) from comment #2)
> Mike, can you take a look?

Yep.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 4 • 12 years ago

(In reply to Mike Habicher [:mikeh] from comment #3)
> (In reply to Lawrence Mandel [:lmandel] from comment #1)
> > basecamp+ assuming that the user is going to need to figure out that they
> > have to kill the audio clip before they can use the camera. ie. When the
> > camera crashes it will continue to crash until the user manually stops the
> > audio stream.
> 
> I don't think the user should have to do this manually.  When I open the
> camera on my BlackBerry, it automatically suspends any playing audio.  (I
> assume this is because the DSP can't support both operations at the same
> time.)

Seems I wasn't clear in my first post. I agree with your statement. What I was trying to say is that this bug is marked basecamp+ assuming that the current scenario requires the user to do this manually. The fix should make this automatic and significantly easier for the user.

	Mike Habicher [:mikeh] (high bugzilla latency)
	Comment 5 • 12 years ago

I don't see this on recent builds:
Gecko: 801806df98e56233382293eb4c4adadbc59772cb
Gaia : 00717bb9304393ee9b38c346f4b97f9bdeeb2d5d

If I start playing a song and then switch to the camera, I can:
1. take a picture
2. switch to video mode
3. start recording a video
4. stop recording a video

When I do this, at (3) the audio playback stops (though apparently not the progress in the track); and at (4), the audio resumes from its current position (i.e. it was not paused between steps 3 and 4).

No crashes observed.

	Inder Reporter
	Comment 6 • 12 years ago

I also noticed that it doesn't crash anymore. But I am noticing that the audio playback doesn't stop while we are recording. It keeps on playing the audio. The expected behavior is the audio should pause when we switch to camcorder mode. So, this is still an issue, right?

I am on following revisions:
Gecko: fbee23615cebc9348a1db4ec1021eb863b98aa2d
Gaia: 5dcc1b1155e8c99b4d8ef738530fb9505a15aee0

	Alex Keybl [:akeybl]
	Comment 7 • 12 years ago

Marking for C2, given this meets the criteria of known P1/P2 blocking-basecamp+ bugs at the end of C1.

	Mike Habicher [:mikeh] (high bugzilla latency)
	Comment 8 • 12 years ago

I think any fix for this will have to happen in gaia.

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 9 • 12 years ago

renom because this bug has been morphed to another bug.

	Tim Taubert [:ttaubert] (inactive)
	Comment 10 • 12 years ago

Looks like we'll need some notification or event that is dispatched when the user starts recording a video. The music app should then pause (or quit?).

	Marco Chen [:mchen] Assignee
	Comment 11 • 12 years ago

Dear all,

According to mechanism from bug 805333, it will monitor all media elements then adjudge who should be muted or paused. But this issue didn't create any media elements so audio channel service introduced from 805333 will not handle this.

A new bug 815069 will be created to build a general mechanism related to bug 805333 for those Apps without media elements to join audio competing policy.

Welcome for any suggestion to that bug.

Thanks.

	Dale Harvey (:daleharvey)
	Comment 12 • 12 years ago

I attempted a workaround that jonas suggested, 

  _silentSound: new Audio('./resources/sounds/silent.wav'),

  this._silentSound.mozAudioChannel = 'content';
  this._silentSound.loop = true;

  this._silentSound.play();

This crashed the video recorder, if I was using headphones the music get sent to the speaker and the headphones were blank, once I stopped recording the music started playing in the headphones again.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 13 • 12 years ago

The automatic silencing of lower-priority channels isn't yet implemented. That's bug 805333. But we still shouldn't crash, we never should, so please file a separate bug on that. Ideally with a crash stack.

	Chris Lee [:clee]
	Comment 14 • 12 years ago

Given this is no longer crashing, marking as P3.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 15 • 12 years ago

renoming as clee has downgraded priority.

	Andrew Overholt [:overholt]
	Comment 16 • 12 years ago

Andrea thinks this may already be fixed now that the changes in bug 805333 have landed.  Can someone verify?

	Marco Chen [:mchen] Assignee
	Comment 17 • 12 years ago

This bug also depends on bug 815069.
It introduced a new component called AudioChannelAgent and used to let FM and Video recording join AudioChannelService. 

Hi Dale,
I plan to use bug 815069 for solving this bug.
May I help here and provide the patch?

Thanks.

	Dale Harvey (:daleharvey)
	Comment 18 • 12 years ago

mchen, happy to

	Marco Chen [:mchen] Assignee
	Comment 19 • 12 years ago

To use the AudioChannelAgent introduced from Bug 815069 for Video Recording to join AudioChannelService.

	Mike Habicher [:mikeh] (high bugzilla latency)
	Comment 20 • 12 years ago

Comment on attachment 689639 [details] [diff] [review]
WIPv1

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

This looks good, but I wonder if it makes more sense to disable audio playback sooner, such as when the camera switches into video recording mode, instead of later when it starts recording.

I think this would provide a better user experience.

Look for getPreviewStreamVideoMode(); the clean-up could go into getPreviewStream() (for when the camera switches to picture mode) and OnNavigation() (for when the camera app is closed).

::: dom/camera/DOMCameraControl.cpp
@@ +264,5 @@
>                         NS_LITERAL_STRING("starting").get());
>  
> +  if (!mAudioChannelAgent) {
> +    nsresult rv;
> +    mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);

Is it okay to ignore rv?

@@ +266,5 @@
> +  if (!mAudioChannelAgent) {
> +    nsresult rv;
> +    mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);
> +    if (mAudioChannelAgent) {
> +      // Camera App will stop recording while it falls to background, so no necessary to hear callback.

"Camera app will stop recording when it falls to the background, so no callback is necessary."

@@ +267,5 @@
> +    nsresult rv;
> +    mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);
> +    if (mAudioChannelAgent) {
> +      // Camera App will stop recording while it falls to background, so no necessary to hear callback.
> +      mAudioChannelAgent->Init( AUDIO_CHANNEL_CONTENT, nullptr);

No space before AUDIO_CHANNEL_CONTENT.

@@ +268,5 @@
> +    mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);
> +    if (mAudioChannelAgent) {
> +      // Camera App will stop recording while it falls to background, so no necessary to hear callback.
> +      mAudioChannelAgent->Init( AUDIO_CHANNEL_CONTENT, nullptr);
> +      // Video Recording doesn't have output sound so no necessary to check canPlay.

"Video recording doesn't output any sound, so it's not necessary to check canPlay."

::: dom/camera/DOMCameraControl.h
@@ +42,5 @@
>    /* additional members */
>    nsRefPtr<ICameraControl>        mCameraControl; // non-DOM camera control
>    nsCOMPtr<nsICameraCapabilities> mDOMCapabilities;
> +  // An agent used to join audio channel service.
> +  nsCOMPtr<nsIAudioChannelAgent> mAudioChannelAgent;

An extra space before mAudioChannelAgent, please.

	Marco Chen [:mchen] Assignee
	Comment 21 • 12 years ago

1. Following the comment.
2. From my point of view, I would like to stop another one's audio as necessary as possible. So I still keep the codes on start/stopRecording(). Of course, if there is another opinion for this, we may pass it to the UX export to do decision.

Thanks.

	Mike Habicher [:mikeh] (high bugzilla latency)
	Comment 22 • 12 years ago

(In reply to Marco Chen [:mchen] from comment #21)
> 
> 2. From my point of view, I would like to stop another one's audio as
> necessary as possible. So I still keep the codes on start/stopRecording().
> Of course, if there is another opinion for this, we may pass it to the UX
> export to do decision.

Josh, can you or someone else in UX comment on stopping/restarting background audio (i.e. the music player) only when the camera starts/stops video recording (as opposed to, say, when the camera enters/leaves video recording mode, or when the camera is opened/closed)?

	Marco Chen [:mchen] Assignee
	Comment 23 • 12 years ago

Add the reviewer and checkin the code first.
If there is any further comment from UX we can follow it on another bug.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 24 • 12 years ago

Let's file a followup if we want to adjust the time when the audio gets muted. It's not obvious to me that that is a blocker.

https://hg.mozilla.org/integration/mozilla-inbound/rev/97c5efcc2115
https://hg.mozilla.org/releases/mozilla-aurora/rev/1e834b77370c
https://hg.mozilla.org/releases/mozilla-beta/rev/7c372d234b13

	Ed Morley [:emorley]
	Comment 25 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/97c5efcc2115

	Al Tsai [:atsai]
	Comment 26 • 12 years ago

wait for code landing to verify

	Askeing Yen[:askeing][:fyen]
	Comment 27 • 12 years ago

https://hg.mozilla.org/releases/mozilla-b2g18/rev/7c372d234b13

	Askeing Yen[:askeing][:fyen]
	Comment 28 • 12 years ago

Build: unagi_2012-12-13_eng.zip from https://releases.mozilla.com/b2g/

The music will stop/resume on start/stop recording, same as Comment 21 said.

	Josh Carpenter [:jcarpenter]
	Comment 29 • 12 years ago

(In reply to Mike Habicher [:mikeh] from comment #22)
> (In reply to Marco Chen [:mchen] from comment #21)
> > 
> > 2. From my point of view, I would like to stop another one's audio as
> > necessary as possible. So I still keep the codes on start/stopRecording().
> > Of course, if there is another opinion for this, we may pass it to the UX
> > export to do decision.
> 
> Josh, can you or someone else in UX comment on stopping/restarting
> background audio (i.e. the music player) only when the camera starts/stops
> video recording (as opposed to, say, when the camera enters/leaves video
> recording mode, or when the camera is opened/closed)?

Deferring to Casey, who is working on media apps and audio issues.

	Casey Yee
	Comment 30 • 12 years ago

I don't see any reason why someone would want to have their music to continue to play in the background while recording video.  Unless it was intentional and was included in the recorded audio.  A neat idea but certainly OOS.

For this reason I think it would make sense to:
When the user starts recording video background audio should be muted/paused until the recording is stopped.   Once stopped the background audio should continue.   

This behavior is consistent with resuming audio after receiving/making a phone call.

Taking photos is a little different.   We will want to continue to play background audio so that the user can continue to listen to music/content while they take photos.   The shutter sound should also be played back so that the user knows when they have taken a photo.

	Inder Reporter
	Comment 31 • 12 years ago

Works fine now.

	Askeing Yen[:askeing][:fyen]
	Comment 32 • 12 years ago

Build ID: 20130107110332

The music will stop/resume on start/stop recording, same as Comment 21/Comment 30 said.

Verified.