Closed Bug 1112410 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

()

VERIFIED FIXED
mozilla38
blocking-b2g 2.1+
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed
b2g-v2.1 --- verified
b2g-v2.1S --- fixed
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: rmitchell, Assigned: sotaro)

References

()

Details

(Keywords: regression, Whiteboard: [2.2-exploratory-2])

Attachments

(7 files)

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
Flags: needinfo?(dharris)
QA Whiteboard: [QAnalyst-Triage?]
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
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
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(dharris) → needinfo?(gchang)
NI NoJun for more information as he's the QA contact for media.
Flags: needinfo?(gchang) → needinfo?(npark)
[Blocking Requested - why for this release]:

This looks like a significant failure to me, noming for 2.1
blocking-b2g: --- → 2.1?
Flags: needinfo?(npark)
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..
blocking-b2g: 2.1? → 2.1+
QA Contact: 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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
pushlog indicates Bug 1082563 - can you take a look Russ?
Blocks: 1082563
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(rnicoletti)
QA Contact: ckreinbring
Assignee: nobody → rnicoletti
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?
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(rnicoletti)
Flags: needinfo?(rmitchell)
RJ is no longer 'with us' - adding QA-Wanted to address his NI from prior comment
QA Whiteboard: [QAnalyst-Triage+]
Flags: needinfo?(rmitchell)
Keywords: qawanted
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: qawanted
QA Contact: ktucker
NI on Russ, can you reproduce on master given the STR in comment 10?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga) → needinfo?(rnicoletti)
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.
Flags: needinfo?(rnicoletti)
(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.
I could reproduce the problem on the STR in comment 10. I am going to investigate the problem from gecko side point of view.
Flags: needinfo?(sotaro.ikeda.g)
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).
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.
Component: Gaia::Video → Video/Audio
Product: Firefox OS → Core
Changed to correct component from Comment 16 and Comment 17.
This problem seems a regression of MediaDecoderStateMachine's asynchronization.
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.
(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 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.
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.
russn, I take this bug since the problem is in gecko side. Thanks.
Assignee: rnicoletti → sotaro.ikeda.g
The patch fixed the problem locally.
attachment 8546114 [details] [diff] [review] can not apply directly to b2g v2.1. attachment 8546114 [details] [diff] [review] has a dependency to Bug 1065827.
Attachment #8546114 - Flags: review?(cpearce)
Attachment #8546114 - Flags: review?(cpearce) → review+
(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.
Blocks: 1050031
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.
(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 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.
Attachment #8547628 - Flags: review?(cpearce)
Priority: -- → P5
Attachment #8547628 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/4568af6d76d3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Please request b2g34 and b2g37 approval on the relevant patches when you get a chance :)
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
Attachment #8546114 - Flags: approval-mozilla-b2g37?
Attachment #8546114 - Flags: approval-mozilla-b2g34?
Keywords: verifyme
Comment on attachment 8546114 [details] [diff] [review]
patch - Handle set dormant during seeking

Can QA please help with branch verification once this lands. Thanks!
Attachment #8546114 - Flags: approval-mozilla-b2g37?
Attachment #8546114 - Flags: approval-mozilla-b2g37+
Attachment #8546114 - Flags: approval-mozilla-b2g34?
Attachment #8546114 - Flags: approval-mozilla-b2g34+
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.
Attachment #8546114 - Flags: approval-mozilla-beta?
Attachment #8546114 - Flags: approval-mozilla-aurora?
I guess the wontfixes were an typo.
Attachment #8546114 - Flags: approval-mozilla-beta?
Attachment #8546114 - Flags: approval-mozilla-beta+
Attachment #8546114 - Flags: approval-mozilla-aurora?
Attachment #8546114 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 1119714
(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.
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
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
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.