Closed
Bug 1020176
Opened 12 years ago
Closed 12 years ago
Audio offload playback not switching to normal playback properly after Pause, BT headset connect and Play
Categories
(Core :: Audio/Video, defect)
Tracking
()
People
(Reporter: vasanth, Assigned: vasanth)
References
()
Details
(Whiteboard: [caf priority: p1][CR 671004])
Attachments
(1 file)
|
1.96 KB,
patch
|
roc
:
review+
cbook
:
checkin+
|
Details | Diff | Splinter Review |
Audio playback doesn't resume in following scenario
1. Play mp3 audio clip in offloaded mode.
2. Pause the clip
3. Connect BT headset
4. Play the clip
Expected behavior
Audio playback should resume in normal mode from where it paused.
Actual behavior:
It doesn't play.
After Pressing Pause -> Play -> it plays from start.
Problem is both AudioOffloadPlayer::ChangeState() Error and AudioOffloadPlayer::NotifyAudioTearDown() are generated in this scenario. I was expecting we will receive only one in this case. So MediaOmxDecoder::ResumeStateMachine gets called twice which is not reentrant.
Attached fix to not call ResumeStateMachine 2nd time, by checking whether mAudioOffloadPlayer is null.
Also moved PlaybackPositionChanged() inside mAudioOffloadPlayer null check, to avoid firing wrong timeupdate event when mAudioOffloadPlayer is null.
[1] http://dxr.mozilla.org/mozilla-central/source/content/media/omx/MediaOmxDecoder.cpp#104
Attachment #8433991 -
Flags: review?(roc)
Attachment #8433991 -
Flags: review?(paul)
Attachment #8433991 -
Flags: review?(roc) → review+
Updated•12 years ago
|
Component: General → Video/Audio
Product: Firefox OS → Core
Version: unspecified → 30 Branch
(In reply to Jason Smith [:jsmith] from comment #1)
> Is this happening in 1.3?
Please note this feature is in 1.4 and above with KitKat only builds
Updated•12 years ago
|
blocking-b2g: 1.4? → 1.4+
Updated•12 years ago
|
Whiteboard: [CR 671004] → [caf priority: p1][CR 671004]
Roc/Paul can any one of you give a try run
Flags: needinfo?(roc)
Flags: needinfo?(paul)
Flags: needinfo?(roc)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> https://tbpl.mozilla.org/?tree=Try&rev=defe72d0f790
Thanks Roc. Among 9 failures, starred 7, other 2 seems unrelated to this change.
Can we add "checkin-needed"?
Flags: needinfo?(roc)
Comment on attachment 8433991 [details] [diff] [review]
resume-once-multiple-errors.patch
Yes!
Attachment #8433991 -
Flags: checkin?
Flags: needinfo?(roc)
Comment 7•12 years ago
|
||
Updated•12 years ago
|
Attachment #8433991 -
Flags: checkin? → checkin+
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•12 years ago
|
Attachment #8433991 -
Flags: review?(paul)
Flags: needinfo?(paul)
Updated•12 years ago
|
status-b2g-v1.4:
--- → affected
status-b2g-v2.0:
--- → fixed
status-firefox30:
--- → wontfix
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
Comment 9•12 years ago
|
||
Comment 10•11 years ago
|
||
This will need a new test case to be covered.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Comment 11•11 years ago
|
||
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/14347/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Flags: in-moztrap+
You need to log in
before you can comment on or make changes to this bug.
Description
•