1112410 - [Video player] tapping fast forward / rewind as user receives call video controls breaks

Status: VERIFIED FIXED

(Core :: Audio/Video, defect, P5)

Description

[[:RJ]]

Attachment: logcat_20141216_1641.txt

Description:
Taping fast forward and rewind up to the point a phone receives a call, after call is ended play fast forward and rewind are broken


Repro Steps:
1) Update a Flame to 20141216040205
2) Launch a video player > play video 
3) Have another device call 
4) As the call is inbound tap fast forward button till call screen is prompted 


Actual:
video is blacked out, play/pause, fast foreword, and rewind are broken 


Expected:
Video plays normally 

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141216040205
Gaia: af3d2f89f391c92667e04676fc0ac971e6021bb7
Gecko: a3030140d5df
Gonk: e5c6b275d77ca95fb0f2051c3d2242e6e0d0e442
Version: 37.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0




Repro frequency: 3/5
See attached: logcat, video clip:https://www.youtube.com/watch?v=RX4JAlGYKbU&feature=youtu.be

Comment 1

[[:RJ]]

Note: this occurs most in landscape mode 

issue was   affected in  2.1 flame
Play fast forward and rewind are broken 

Flame 2.1

Environmental Variables:
Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141216001202
Gaia: 79286eafe67707d1330966c1b1413b2d0de595d9
Gecko: 222a62b532db
Gonk: e5c6b275d77ca95fb0f2051c3d2242e6e0d0e442
Version: 34.0 (2.1)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

does not occur on 2.0 

Flame 2.0
this appears to be a landscape issues, there was no landscape on 2.0

Environmental Variables:
Device: Flame 2.0 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141216000202
Gaia: f3b9806f687fbbd7eba6b0e1f6ebb8bde09840ea
Gecko: 12aea1649f5a
Gonk: e5c6b275d77ca95fb0f2051c3d2242e6e0d0e442
Version: 32.0 (2.0)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This repro of this bug is 3/5 and doesnt seem like a high use case, but looks very bad to the end user. NI windows management owner for blocking decision

Comment 3

[Gerry Chang [:gchang]]

NI NoJun for more information as he's the QA contact for media.

Comment 4

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:

This looks like a significant failure to me, noming for 2.1

Comment 5

[bhavana bajaj [:bajaj]]

Can QA help with the regression range here ? NI :Hema as well to help with this, blocking on this due to a couple of issues : buttons are broken and the orientation is not as expected, screen goin black etc..

Comment 6

[Chris Kreinbring [:CKreinbring]]

Regression window
Last working
BuildID: 20141021174634
Gaia: 82174cee5ede9f23aedad8a39f8b8cdc1ae710c4
Gecko: 367d8d88c2cb
Platform Version: 36.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First broken
BuildID: 20141021184730
Gaia: 4d7f051cede6544f4c83580253c743c22b0cb279
Gecko: 580073ea1544
Platform Version: 36.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Working Gaia / Broken Gecko = No repro
Gaia: 82174cee5ede9f23aedad8a39f8b8cdc1ae710c4
Gecko: 580073ea1544
Broken Gaia / Working Gecko = Repro
Gaia: 4d7f051cede6544f4c83580253c743c22b0cb279
Gecko: 367d8d88c2cb
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/82174cee5ede9f23aedad8a39f8b8cdc1ae710c4...4d7f051cede6544f4c83580253c743c22b0cb279

Comment 7

[Joshua Mitchell (Inactive)]

pushlog indicates Bug 1082563 - can you take a look Russ?

Comment 8

[Russ Nicoletti [:russn]]

I tried more than 10 times following the STR but I cannot reproduce the issue. I tried v2.1 BuildID 20141216001202 and also with the latest master build. In any event, I would like to know if using the forward/rewind functionality is necessary to recreate the issue. Either way, it's possible there is an issue with reloading the video on a visibility change. This is the functionality that was changed by bug 1082563; the change was the video app no longer explicitly unloads and loads the video when going to the background and coming back to the foreground because gecko is handling that.

RJ, can you try recreating without the forward/rewind tapping? It would be helpful to know if that is necessary to recreate this.

Sotaro, can you take a look at the attached video and logcat and comment on whether there may be a gecko issue re-loading the video when the video app comes back to the foreground after the call is dismissed?

Comment 9

[Joshua Mitchell (Inactive)]

RJ is no longer 'with us' - adding QA-Wanted to address his NI from prior comment

Comment 10

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

I was only able to reproduce this issue on the Flame 2.2 by pressing and holding down the rewind, fast forward or slider buttons while watching a video until the call screen appears.

1. Launch the video app and play a video.
2. While the video is playing, press and hold down the rewind or fast forward button. 
3. Have another phone call the test phone.
4. Take your finger off the rewind or fast forward button once the call screen appears.
5. Tap the red button to end the call and then try to play the video again.

Actual: 
The video controls will no longer work and the screen will be blank instead of showing the video.

Environmental Variables:
Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20150106010234
Gaia: b77e0d56d197e0ee02d801a25c784130d888c9db
Gecko: 2a193b7f395c
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
  
Repro frequency: 100% 5/5 attempts

Comment 11

[Peter Bylenga [:PBylenga]]

NI on Russ, can you reproduce on master given the STR in comment 10?

Comment 12

[Russ Nicoletti [:russn]]

Attachment: logcat, issue reproduces

I was able to reproduce using the STR in comment 10, although not consistently. In any event, I'm attaching the logcat for when the issue reproduced and also a logcat for when it did not reproduce. However, I don't see any obvious difference between the two. 

I need to do more investigation to figure out whether there is something weird going in with gecko or that app.

Comment 13

[Russ Nicoletti [:russn]]

Attachment: logcat, issue does not reproduce

Comment 14

[Russ Nicoletti [:russn]]

(In reply to Russ Nicoletti [:russn] from comment #12)
> 
> I need to do more investigation to figure out whether there is something
> weird going in with gecko or that app.

I need to do more investigation to see whether or not the video has been reloaded and if so whether the current position is correct. That is, it still needs to be determined if this is a gecko issue or a video app issue.

Comment 15

[Sotaro Ikeda [:sotaro]]

I could reproduce the problem on the STR in comment 10. I am going to investigate the problem from gecko side point of view.

Comment 16

[Russ Nicoletti [:russn]]

Sotara, here is what I've found from my investigating:

When the phone call is dismissed and when the video app comes to the foreground, the video app gets a 'loadedmetdata' event -- indicating the video has been reloaded -- but it does not get a 'timeupdated' event like it normally does when the video app comes back to the foreground after having been in the background. 

When the video app comes back to the foreground, the ‘seeking’ property of the video element remains true because the app does not get a ‘seeked’ event. 

When the issue does not reproduce, the video app after coming back to the foreground gets the ‘loadedmetadata’, ‘timeupdated’, and ‘seeked’ events (at which point of course the ‘seeking’ property has been set to false).

Comment 17

[Sotaro Ikeda [:sotaro]]

russn, thanks for the investigation! I am still in the middle of investigation. But the problem seems to be caused by MediaDecoderStateMachine's problem. When the problem happened incorrect state transition was observed.

Comment 18

[Sotaro Ikeda [:sotaro]]

Changed to correct component from Comment 16 and Comment 17.

Comment 19

[Sotaro Ikeda [:sotaro]]

This problem seems a regression of MediaDecoderStateMachine's asynchronization.

Comment 20

[Sotaro Ikeda [:sotaro]]

When the problem happened, MediaDecoderStateMachine::SetDormant(true) was called, MediaDecoderStateMachine's state did not stay DECODER_STATE_DORMANT state, it soon backed to DECODER_STATE_DECODING without MediaDecoderStateMachine::SetDormant(false) call.

Comment 21

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #20)
> When the problem happened, MediaDecoderStateMachine::SetDormant(true) was
> called, MediaDecoderStateMachine's state did not stay DECODER_STATE_DORMANT
> state, it soon backed to DECODER_STATE_DECODING without
> MediaDecoderStateMachine::SetDormant(false) call.

Sorry, yesterday's investigation was incorrect.

Comment 22

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #21)
> (In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #20)
> > When the problem happened, MediaDecoderStateMachine::SetDormant(true) was
> > called, MediaDecoderStateMachine's state did not stay DECODER_STATE_DORMANT
> > state, it soon backed to DECODER_STATE_DECODING without
> > MediaDecoderStateMachine::SetDormant(false) call.
> 
> Sorry, yesterday's investigation was incorrect.

In my log, there were multiple instances of MediaDecoderStateMachine logout and misunderstood it.

Comment 23

[Sotaro Ikeda [:sotaro]]

The problem was happened because of conflict between seek and dormant. In current implementation, if dormant happen during seeking, the seeking status is cleared when exiting dormant state.

Comment 24

[Sotaro Ikeda [:sotaro]]

russn, I take this bug since the problem is in gecko side. Thanks.

Comment 25

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Handle set dormant during seeking

The patch fixed the problem locally.

Comment 26

[Sotaro Ikeda [:sotaro]]

attachment 8546114 [details] [diff] [review] can not apply directly to b2g v2.1. attachment 8546114 [details] [diff] [review] has a dependency to Bug 1065827.

Comment 27

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3bfa75b73e7a
https://tbpl.mozilla.org/?tree=Try&rev=3bfa75b73e7a

Comment 28

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #26)
> attachment 8546114 [details] [diff] [review] can not apply directly to b2g
> v2.1. attachment 8546114 [details] [diff] [review] has a dependency to Bug
> 1065827.

It seems better to avoid uplifting whole Bug 1065827 fix to b2g v2.1. The change is too big. Need to minimize the change.

Comment 29

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2g v2.1 - Handle set dormant during seeking

A patch for b2g v2.1. Code of b2g v2.1 has some difference from master. Therefore the patch is also different from the patch for master.

Comment 30

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #29)
> Created attachment 8547628 [details] [diff] [review]
> patch for b2g v2.1 - Handle set dormant during seeking
> 
> A patch for b2g v2.1. Code of b2g v2.1 has some difference from master.
> Therefore the patch is also different from the patch for master.

attachment 8547628 [details] [diff] [review] has some code from a fix of Bug 1065827.

Comment 31

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8547628 [details] [diff] [review]
patch for b2g v2.1 - Handle set dormant during seeking

cpearce, can you review the patch? The patch for b2g v2.1 is a bit different from a patch for master.

Comment 32

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5decef26d9e2
https://tbpl.mozilla.org/?tree=Try&rev=5decef26d9e2

Comment 33

[Sotaro Ikeda [:sotaro]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4568af6d76d3

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4568af6d76d3

Comment 35

[Ryan VanderMeulen [:RyanVM]]

Please request b2g34 and b2g37 approval on the relevant patches when you get a chance :)

Comment 36

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8546114 [details] [diff] [review]
patch - Handle set dormant during seeking

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 871485 
User impact if declined: failed to playback video in some use cases.
                         It continue until an application exit.
Testing completed: locally done.
Risk to taking this patch (and alternatives if risky): low risk.
String or UUID changes made by this patch: none

Comment 37

[bhavana bajaj [:bajaj]]

Comment on attachment 8546114 [details] [diff] [review]
patch - Handle set dormant during seeking

Can QA please help with branch verification once this lands. Thanks!

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e45d664da0df
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/9743e85c0224

Comment 39

[Ralph Giles (:rillian)]

Comment on attachment 8546114 [details] [diff] [review]
patch - Handle set dormant during seeking

MSE team would like this on 37 and 36 to reduce variance in dom/media during feature uplift.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing; more difficult to apply fixes later in the release cycle.
[Describe test coverage new/current, TBPL]: Landed on m-c.
[Risks and why]: This affects non-MSE playback, but is an isolated change.
[String/UUID change made/needed]: None.

Comment 40

[Sylvestre Ledru [:Sylvestre]]

I guess the wontfixes were an typo.

Comment 42

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/9743e85c0224

Comment 43

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-beta/rev/5d185a7d03b5

Comment 44

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ee5c1552ca19

Comment 45

[Ryan VanderMeulen [:RyanVM]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #40)
> I guess the wontfixes were an typo.

I usually set them on B2G uplifts when it looks like they aren't wanted for Fx (judging by affected product, lack of approval requests, etc). In this case, it would have been good if they'd been set back to affected at the time of the approval request.

Comment 46

[Shally Li（leave from Mozilla）]

This bug has been successfully verified on Flame v2.1 by the steps in Comment 0 & Comment 10.
See attachment: verified_v2.1.mp4 and verified_v2.2.mp4.
Reproduce rate: 0/6.

Flame v2.1 build:
Gaia-Rev        77c57eb8a985d5cbd34a597fb1b978ba6e205af6
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/452a023ae7b2
Build-ID        20150119001222
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150119.035259
FW-Date         Mon Jan 19 03:53:10 EST 2015
Bootloader      L1TC000118D0

Flame v2.2 build: 
Gaia-Rev        f5b3d1b6cfa3e702033f613915ae637cb735cbfb
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/bccee1a13ba6
Build-ID        20150119002502
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150119.035831
FW-Date         Mon Jan 19 03:58:42 EST 2015
Bootloader      L1TC000118D0

Comment 47

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4

Comment 48

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.2.MP4

Comment 49

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

This issue has been verified as fixed on Flame 3.0

The video controls work as expected after pressing the rewind or fast forward buttons then receiving and ending a call.

Environmental Variables:
Device: Flame 3.0(Full Flash)(KK)(319mb)
BuildID: 20150121010204
Gaia: 5e98dc164b17fd6decb48a9eaddef0e55b82e249
Gecko: 540077a30866
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0