Closed Bug 1120612 Opened 9 years ago Closed 6 years ago

Ringtones app doesn't release audio channel when launching child activities

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: squib, Assigned: squib)

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
djf
: review+
squib
: ui-review?
tif
Details | Review
Short version: yet another issue where our grody audio competing policy workarounds have bitten us.

If you preview a ringtone in the ringtones manager and then launch a child activity, we still hold a lock on the "ringtone" audio channel, which means that any attempts to change the volume during the child activity will affect the ringtone volume, not whatever audio channel is currently playing.
Attached file Fix it
This is a pretty simple patch. I just make sure that any time we call stop(), we release the audio channel too. We were already effectively doing this most of the time, but now we also do it when launching child activities from the ringtone manager.

I also slightly changed when we stop playing a preview when we open the actions menu. Originally, we stopped immediately when it popped up. Now, we stop when an action is performed; deleting just stops it, and sharing stops and releases the audio channel. Cancelling out of the actions menu lets the preview continue playing.

Of course, all this is really hard to test, given that bug 1107534 still hasn't been resolved...
Attachment #8547767 - Flags: ui-review?(tshakespeare)
Attachment #8547767 - Flags: review?(dflanagan)
Hey Jim! Haven't looked at the patch yet, but wanted to ask if there's a technical reason for the change mentioned in paragraph #2. If not, I would like to keep the behaviour the same - ringtone preview stops on action menu.

Thanks!
I can do it either way, but this way is a little easier. I actually think it's better too, since tapping the action menu and immediately cancelling won't interrupt the preview. That's more in line with how I'd expect this to work as a user: the preview only stops when the ringtone manager is closed/hidden (or the file is deleted). Stopping the preview just because you opened a menu is strange.
I see your point, but in this case of previewing the ringtone it should stop when the action menu is launched. 

I know it's harder but I have total faith that you can pull it off! :) Thanks Jim!
Do you have a reason for why it should be that way (aside from it being that way now)? My vague recollection was that we did it the old way because it was easier back then, not because it was the "ideal" UX.
Nope - that was the way it was designed and how we wanted it to behave. There was a bug somewhere that actually fixed the behaviour because it wasn't always this way.

In this context, the user's primary task at first is to "preview ringtone" - "is this the one I wanted?". Once the user decides to take action on that specific ringtone, they no longer need to hear the ringtone, their primary task has shifted to something else. 

The same is true of a video - which was another bug that I believe got fixed. If the video is playing, the user is focused on that task. Opening the action menu would then pause/stop the video because the user's attention has moved away to a different task.

Note that both of these situations differ from music which is supposed to continue playing in the background even when the user's attention is not focused on the music app. That is because listening isn't the user's primary task and it's ok if they tune out what is playing while focusing on a different task e.g. replying to an email.

Enjoy!
Ok, I've adjusted it to stop playback when the action menu opens, but then it closes the audio channel when the share starts. I'm still not convinced this is the most natural UX, but I don't really have that strong an opinion either.
Thanks Jim! I'm having a couple technical difficulties with my phone. I'll check out the patch tomorrow if I can't get it fixed today.
Hey Jim - I can't actually hear anything when I tap on a ringtone. I've got all the volumes way up and can hear sound when I fiddle with the sliders in the sound setting, but I can't hear anything in the Alert, Ringtone, or Manage Tones screens.

I've updated to the lasted base image and builds.
That's because it's broken. See bug 1107534.
hmmm seems bad and makes a UI review difficult. LMK when it's working again and I'll review the patch. Thanks!
Comment on attachment 8547767 [details] [review]
Fix it

This patch looks okay to me, but I have not tested it myself.

I'm okay with this landing as is, but I wonder if it could be simpler. As far as I can tell TonePlayer.setTone() always calls setExclusiveMode(true). So could you must modify TonePlayer.stop() so that it always calls setExclusiveMode(false)?  If you did that, I think most of the rest of this patch would no longer be necessary.  I see there is one place in this patch where you are calling stop(false).  Maybe there is some reason that I don't understand why you really don't want to give up the audio channel in that case?
Attachment #8547767 - Flags: review?(dflanagan) → review+
(In reply to David Flanagan [:djf] from comment #12)
> I see there is one place in this patch
> where you are calling stop(false).  Maybe there is some reason that I don't
> understand why you really don't want to give up the audio channel in that
> case?

If we give up the audio channel then, music in the background can start playing again, before we're done managing the ringtones. I wanted to avoid resuming the background audio when it's likely that the user is still going to do things in the manager.
Firefox OS is not being worked on
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: