1116741 - [Flame][Video]Cancel sharing a video and return to Video app,"Cannot play video:" alert message is shown on screen for 1 second.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Video, defect)

Description

[Elie]

[1.Description]:
[Flame2.2][Video]Cancel sharing a video and return to Video app,"Cannot play video:" alert message is shown on screen for 1 second.
Attchment:logcat.txt  and  VIDEO0195_Compress.MP4
Happen time:4:38

[2.Testing Steps]: 
Precondition:Don't login any email account.
1.Kill the Video and Email in Recent apps
2.Launch Video and play a video
3.Tap ... button at top right corner when playing
4.tap Share->Email
>>Comfirmation view pops up,
5.press cancel

[3.Expected Result]: 
5.It returns to Video view and will not pop up "Cannot play video: Another app is currently using the video player" 

[4.Actual Result]: 
5.It returns to Video view and pops up "Cannot play video: Another app is currently using the video player" for  1 second.

[5.Reproduction build]: 
Flame 2.2 build:
Gaia-Rev        322ef5116a5827a30c9a3cd9b842449a9c66a5b3
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/67872ce17918
Build-ID        20141230010205
Version         37.0a1

[6.Reproduction Frequency]: 
occasionally Recurrence,5/10
TCID: Free Test

Comment 1

[Elie]

Attachment: logcat.txt

Comment 2

[Elie]

Attachment: VIDEO0195_Compress.MP4

Comment 3

[Eric Chang [:ericcc] [:echang]]

Attachment: 2015-01-06-23-19-09.png

I got different result as yours, "Cannot play video: Another app is currently using the video player" is not shown, but I got keyboard pops up there..

Comment 4

[Shine(Leave from Mozilla)]

Attachment: logcat

Hi Eric,
This problem cannot be repro on latest build of Woodduck 2.0/Flame2.0, 2.1, 2.1s(512mb), 3.0.  But it can be repro on Flame 2.2. I got the same result as reporter and keyboard haven't popped up. Could you please help with it? thanks!
See attachments: 0539_logcat.txt & Flame2.2_Video.MP4
Occurrence time: 05:39
Rate: 5/5

Woodduck 2.0 build:
Build ID               20150302050313
Gaia Revision          a1b5959728c8bc2a82354e197bb161922d419866
Gaia Date              2015-02-13 09:00:02
Gecko Revision         d9b299dc1087f23c83321b4dccc92e0f52309e8e
Gecko Version          32.0
Device Name            jrdhz72_w_ff
Firmware(Release)      4.4.2
Firmware(Incremental)  1425243887
Firmware Date          Mon Mar  2 05:05:08 CST 2015

Flame 2.0 build: 
Build ID               20150301000203
Gaia Revision          366aaa19ac474dc58b79d62a91cff41756ae9dfe
Gaia Date              2015-02-22 20:25:01
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/1bd33f5447d2
Gecko Version          32.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150301.034812
Firmware Date          Sun Mar  1 03:48:22 EST 2015
Bootloader             L1TC000118D0

Flame 2.1 build:
Build ID               20150301161204
Gaia Revision          5d3479fdd438412adee4452720856b6b771fe5cd
Gaia Date              2015-02-25 18:20:09
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/9bf4c663241f
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150301.194919
Firmware Date          Sun Mar  1 19:49:29 EST 2015
Bootloader             L1TC000118D0

Flame 2.2 build:
Build ID               20150301002505
Gaia Revision          77609916ca5ab721150fab2b7bc5c37f43ee3a5a
Gaia Date              2015-02-27 16:35:06
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/27ab8aa34201
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150301.040042
Firmware Date          Sun Mar  1 04:00:53 EST 2015
Bootloader             L1TC000118D0

Flame 3.0 build:
Build ID               20150301010223
Gaia Revision          f34ce82a840ad3c0aed3bfff18517b3f6a0eb37f
Gaia Date              2015-02-27 15:48:31
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/eea6188b9b05
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150301.045059
Firmware Date          Sun Mar  1 04:51:10 EST 2015
Bootloader             L1TC000118D0

2.1S_512m build:
Build ID               20150301161204
Gaia Revision          a43d64ae01ef108aa4dcc971c770fecd8416a764
Gaia Date              2015-02-26 09:24:39
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/2437280c634f
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2
Firmware(Incremental)  122
Firmware Date          Thu Feb  5 12:42:58 CST 2015

Comment 5

[Shine(Leave from Mozilla)]

Attachment: Video

Comment 6

[Eric Chang [:ericcc] [:echang]]

Attachment: Screenshot_2015-03-12-16-50-41.png

I got different popup here, but in either case, it seems to be not quite right here. 
http://youtu.be/Az194Q8yxvo

Comment 7

[Eric Chang [:ericcc] [:echang]]

[Blocking Requested - why for this release]: regression

Comment 8

[Hema Koka [:hema]]

Russ, could you please investigate?

Thanks
Hema

Comment 9

[Russ Nicoletti [:russn]]

I'm not able to reproduce but what seems to be happening is when the sharing is cancelled it is taking the video more than the "allowable" amount of time to load when the video app comes to the foreground. The video app shows the "Cannot play video" dialog when it is unable to load a video within a predetermined amount of time (initially implemented as 1000ms). This functionality was introduced by bug 1079519 to handle situations where another application is using the hardware video codec resulting in the video app not being able to load a video. 

It is an imperfect solution because there are some situations where it takes the video longer than 1000ms to load and thus the dialog displays for some period of time before going away. When implementing this solution, the thinking was this side affect of sometimes (presumably rarely) displaying of the dialog was the price to pay for letting users know the video app was not able to load a video when the video codec was in use by a another app. Bug 1093283 is tracking support for the html video element to indicate when it can't play because the video codec is in use, which is clearly a better solution.

The "display dialog when video cannot be loaded for some predetermined amount of time" functionality was introduced in v2.1, where the wait period was 1000ms. In 2.2 the wait period is also 1000ms. In master the wait period is 1500ms.

My suggestion is instead of fiddling with an imperfect solution, we should focus and getting bug 1093283 landed so that we can have a more robust solution of a video not being able to be played because the video codec is in use; one that doesn't have these undesirable side affects.

Comment 10

[David Flanagan [:djf]]

At this point, even if we are able to get a gecko-based fix, we are probably out of time to modify the video app to use that new gecko feature.

Russ, would you prepare a patch that just increases the wait from 1000 to 1500ms to match what is on master?

Then, we'll at least have some kind of patch that we can ask the reporter to verify.

Comment 11

[Russ Nicoletti [:russn]]

Attachment: Link to github PR: https://github.com/mozilla-b2g/gaia/pull/28934

This PR increases the timeout in 2.2 to 1500ms to match master

Comment 12

[Russ Nicoletti [:russn]]

Comment on attachment 8586480 [details] [review]
Link to github PR: https://github.com/mozilla-b2g/gaia/pull/28934

This PR replaces the existing PR: https://github.com/mozilla-b2g/gaia/pull/29297

Comment 13

[David Flanagan [:djf]]

Comment on attachment 8586480 [details] [review]
Link to github PR: https://github.com/mozilla-b2g/gaia/pull/28934

looks good to me.

Elie: would you check whether this patch resolves the problem for you. This is a workaround rather than a real fix but it may be the best we can do for v2.2

Comment 14

[Elie]

(In reply to David Flanagan [:djf] from comment #13)
> Comment on attachment 8586480 [details] [review]
> Link to github PR: https://github.com/mozilla-b2g/gaia/pull/28934
> 
> looks good to me.
> 
> Elie: would you check whether this patch resolves the problem for you. This
> is a workaround rather than a real fix but it may be the best we can do for
> v2.2
I thinks its OK for me.
Verified in build Flame 2.2 image of patch(commnet 13)
Build ID               20150403140106
Gaia Revision          022eeb91197ba4a9adfd67bd6db5aa03cc69eb31
Gaia Date              2015-04-03 04:13:03
Gecko Revision         n/a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.rose.20150310.100223
Firmware Date          2015/3/10 10:02:48 CST
Bootloader             L1TC000118D0
Reproduce rate         0/5

Comment 15

[David Flanagan [:djf]]

Russ,

Let's get this landed today. Please attach the correct version of the patch to the bug and mark the other one as obsolete. Then set the uplift request flag for :bajaj. Then see if you can get the tests to run again. It looks like there was a lot of orange in this test run. (But maybe that is just because the 2.2 tree has broken tests. This patch clearly isn't breaking things.)

Comment 16

[GitHub Autolander Bot]

Attachment: [gaia] russnicoletti:bug-1116741-v2.2 > mozilla-b2g:v2.2

Comment 17

[Russ Nicoletti [:russn]]

Attachment: Link to github PR: https://github.com/mozilla-b2g/gaia/pull/29332

Previous PRs were against the main gaia repo, not my fork. This PR is against my gaia repo fork.

Comment 18

[David Flanagan [:djf]]

Comment on attachment 8588118 [details] [review]
Link to github PR: https://github.com/mozilla-b2g/gaia/pull/29332

Looks good, Russ.

Comment 19

[David Flanagan [:djf]]

Comment on attachment 8588118 [details] [review]
Link to github PR: https://github.com/mozilla-b2g/gaia/pull/29332

This is a trivial patch that changes two timeouts from 1000ms to 1500ms. We already have 1500ms on master and this just modifies 2.2 to be the same as master. The reporter indicates that this seems to workaround the bug. The only downside is that if the user tries to use the video app while they have a Hello video call in the background, they will have to wait a half second longer before they see the error message indicating that the video can't be played. 

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): this bug is a side effect of the patch we had to put in place to make the video app report an error when a Hello video call was going on in the background. In order to fix this for real, we need a gecko change in bug 1093283, but that does not look like it will arrive in time for 2.2.

[User impact] if declined: the user will see a transient spurious error message if the share a video and then cancel the share.

[Testing completed]: by the bug reporter

[Risk to taking this patch] (and alternatives if risky): not risky at all; it only changes the delay for the error message. As described above, it means that when the error message is not spurious it will take a little longer to see it. But we've been running with this on master for quite some time and it seems to be fine.

[String changes made]: none

Comment 20

[GitHub Autolander Bot]

Pull request has landed in v2.2: https://github.com/mozilla-b2g/gaia/commit/3e0500e945d5f169adc69a41b2dc93a1de955964

Comment 21

[Elie]

This issue verified successfully on flame 2.2:
Flame 2.2:
Build ID               20150407162504
Gaia Revision          ea735c21bfb0d78333213ff0376fce1eac89ead6
Gaia Date              2015-04-07 20:58:15
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/3f86ddb7f719
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Reproduce rate         0/5