1053186 - [MADAI] AudioOffloadPlayer: Glitch observed during mp3 clip startup

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[vasanth]

Attachment: AudioOffloadPlayer-avoid-startup-glitch.patch

[Blocking Requested - why for this release]: Customer issue

Minor glitch observed from 0 to 0.2 seconds during playback of mp3 clip in offlaod mode.

Analysis
--------
1. We figure out whether platform supports offloading in MediaOMXReader [1]
2. Based on that info, MediaOmxDecoder pauses the state machine and starts AudioOffloadPlayer [2]
3. In between [1] and [2], if MediaDecoderStateMachine::StartPlayback() is started, then minor glitch is observed while switching from non offlaod to offload mode.

Suggested Fix
-------------
Attached patch (v1.4) makes MediaDecoderStateMachine::StartPlayback() a noop if  audio offloading is possible to avoid the glitch. 

Will upload V2.0 fix once the V1.4 fix is accepted.

[1] http://dxr.mozilla.org/mozilla-central/source/content/media/omx/MediaOmxReader.cpp#457
[2] http://dxr.mozilla.org/mozilla-central/source/content/media/omx/MediaOmxDecoder.cpp#79

Comment 1

[Wayne Chang [:wchang]]

vasanth:
I think this is better going straight to 2.0 since that's where this is required by the launching partner.
1.4 does not need this.

Comment 2

[vasanth]

Attachment: AudioOffloadPlayer-avoid-startup-glitch.patch

Updated patch sets mFallbackToStateMachine to true in one more error case.

Comment 3

[vasanth]

(In reply to Wayne Chang [:wchang] from comment #1)
> vasanth:
> I think this is better going straight to 2.0 since that's where this is
> required by the launching partner.
> 1.4 does not need this.

Customer raised this issue to us on 1.4 
Moving ni to him

Comment 4

[Michael Vines [:m1] [:evilmachines]]

(v2.0 would be where this is needed ultimately, we don't really need this to land in upstream v1.4)

Comment 5

[Yeojin Chung [:YeojinC]]

No STR is present to create test case to address bug.

Comment 6

[vasanth]

(In reply to Yeojin Chung [:YeojinC] from comment #5)
> No STR is present to create test case to address bug.

1. In MSM platforms (Ex: Flame, QRD8x10) play any .mp3 music file.
2. It would result in offload playback (From logcat, AuidoFlinger will show compress-offload-playback logs. Not sure if there is any other way to find this)
3. Without this patch we can hear a minor glitch during song start up. You can spot the difference by playing same clip with this path or in VLC.

Let me know if you need more info.

(In reply to Michael Vines [:m1] [:evilmachines] from comment #4)
> (v2.0 would be where this is needed ultimately, we don't really need this to
> land in upstream v1.4)

Thanks! Will upload a patch for 2.0

Comment 7

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added in moztrap:

https://moztrap.mozilla.org/manage/case/14339/

Comment 8

[vasanth]

Attachment: AudioOffloadPlayer-avoid-startup-glitch-V2.0.patch

Comment 9

[vasanth]

Attachment: AudioOffloadPlayer-avoid-startup-glitch-V2.1.patch

Rebased the patch for V2.0 AND V2.1

Comment 10

[vasanth]

Try runs
2.1 - https://tbpl.mozilla.org/?tree=Try&rev=3bdecf4857b0
2.0 - https://tbpl.mozilla.org/?tree=Try&rev=f84e756094a4

Comment 11

[vasanth]

(In reply to vasanth from comment #10)
> 2.1 - https://tbpl.mozilla.org/?tree=Try&rev=3bdecf4857b0
2.1 try build failed. My bad with using hg.

Started new 2.1 try run https://tbpl.mozilla.org/?tree=Try&rev=8e9d579217a1

Comment 12

[vasanth]

Try runs seems good now. starred failures. Adding checkin-needed

Comment 13

[vasanth]

Patch for Mozilla-Central - AudioOffloadPlayer-avoid-startup-glitch-V2.1.patch
Patch for 2.0 -  AudioOffloadPlayer-avoid-startup-glitch-V2.0.patch

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f6ea1c97cc6

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9f6ea1c97cc6

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2b622e00d27a