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)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: squib, Assigned: squib)
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 1•9 years ago
|
||
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)
Comment 2•9 years ago
|
||
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!
Assignee | ||
Comment 3•9 years ago
|
||
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.
Comment 4•9 years ago
|
||
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!
Assignee | ||
Comment 5•9 years ago
|
||
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.
Comment 6•9 years ago
|
||
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!
Assignee | ||
Comment 7•9 years ago
|
||
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.
Comment 8•9 years ago
|
||
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.
Comment 9•9 years ago
|
||
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.
Assignee | ||
Comment 10•9 years ago
|
||
That's because it's broken. See bug 1107534.
Comment 11•9 years ago
|
||
hmmm seems bad and makes a UI review difficult. LMK when it's working again and I'll review the patch. Thanks!
Comment 12•9 years ago
|
||
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+
Assignee | ||
Comment 13•9 years ago
|
||
(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.
Comment 14•6 years ago
|
||
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.
Description
•