Closed Bug 1167545 Opened 9 years ago Closed 9 years ago

[Video]The play control button is unavailable while we play second video file from Notifications.

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.5+, b2g-v2.2 affected, b2g-master verified)

VERIFIED FIXED
blocking-b2g 2.5+
Tracking Status
b2g-v2.2 --- affected
b2g-master --- verified

People

(Reporter: huayu.li, Assigned: pdahiya)

Details

(Whiteboard: [2.5-aries-test-run-1] [polish])

Attachments

(4 files)

[1.Description]:
[Flame2.2&3.0][Nexus5 2.2&3.0]While there is a video from BT is playing, we play another video file from BT, the play/pause button of the second video is unavailable.
Fond at:18:58
See attachment:video 1.mp4 and logcat1.txt


[2.Testing Steps]: 
1.There are two devices and connect them with BT.
2.Send two videos from A to B.
3.After two videos are recieved on B, open notification and play the first video.
4.Open notification and play the second video[It makes no difference whether the first video is over or not.]
5.Tap pause button.
6.Wait for the second video to complete playing, Tap play button.

[3.Expected Result]: 
5&6. The play/pause button is available.

[4.Actual Result]: 
5&6. The play/pause button is unavailable..

[5.Reproduction build]: 
Device Flame 2.2(Affected):
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.040918
Firmware Date          Thu May 21 04:09:27 EDT 2015
Bootloader             L1TC000118D0

Device Flame 3.0(Affected):
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.193021
Firmware Date          Thu May 21 19:30:31 EDT 2015
Bootloader             L1TC000118D0

Device Nexus 5_2.2(Affected):
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.040628
Firmware Date          Thu May 21 04:06:43 EDT 2015
Bootloader             HHZ12f

Device Nexus 5_3.0(Affected):
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.193039
Firmware Date          Thu May 21 19:30:54 EDT 2015
Bootloader             HHZ12f

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test
Attached video 1.mp4
Attached file logcat1.txt
Hi Eric, could you help to check this issue?
Flags: needinfo?(echang)
Hi Alissa, Could you also check that, this might not be bluetooth related, try to download 2 video files from the web, and then the notification as you did to see if this still can be reproduced, thanks a lot!
Flags: needinfo?(echang) → needinfo?(huayu.li)
(In reply to Eric Chang [:ericcc] [:echang] from comment #4)
Hi Eric, thanks for your help, same as you assumed, this is not bluetooth related, I download two videos from web or email, and then I open them in notification, this issue also exist.
Flags: needinfo?(huayu.li)
Summary: [Video]The play control button is unavailable while we play second video file from BT. → [Video]The play control button is unavailable while we play second video file from Notifications.
ni?ing hema for assessment.  I think it is a blocking issue since the control is unavailable, but also at the same time not sure how common this scenario is.
Flags: needinfo?(hkoka)
Can the second video never be played when two videos are transferred via bluetooth?

Is this still happening in the latest build?
Flags: needinfo?(hkoka)
ni?ing Alissa for comment 7
Flags: needinfo?(huayu.li)
(In reply to Hema Koka [:hema] from comment #7)
> Can the second video never be played when two videos are transferred via
> bluetooth?
> 
> Is this still happening in the latest build?

Hi Hema&Park, the second video can be played after we tap from notification, just the play/pause button is unusable, and this issue still exist on latest build of FlameKK2.2, FlameKK2.5,AriesKK2.5.

Device:FlameKK_v2.5
Build ID               20150824150208
Gaia Revision          d7fb5717d3e0153ac64af2c0d5c11079846d81c3
Gaia Date              2015-08-24 10:07:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/ba43a48d3c528cc956335793e02504e5ca2c149f
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150824.182403
Firmware Date          Mon Aug 24 18:24:15 EDT 2015
Base Image             v18D v4
Bootloader             L1TC000118D0

Device: FlameKK_v2.2
Build ID               20150824032503
Gaia Revision          335cd8e79c20f8d8e93a6efc9b97cc0ec17b5a46
Gaia Date              2015-08-14 19:06:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1effc4cb6414
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150824.065639
Firmware Date          Mon Aug 24 06:56:50 EDT 2015
Base Image             v18D v4
Bootloader             L1TC000118D0

Device:AriesKK_v2.5
Build ID               20150824124704
Gaia Revision          d7fb5717d3e0153ac64af2c0d5c11079846d81c3
Gaia Date              2015-08-24 10:07:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/ba43a48d3c528cc956335793e02504e5ca2c149f
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150824.122007
Firmware Date          Mon Aug 24 12:20:15 UTC 2015
Bootloader             s1
Flags: needinfo?(huayu.li)
Whiteboard: [2.5-aries-test-run-1]
Should the non-functioning button enough to raise this as a blocker?  I'm sort of on the fence on this one.
Flags: needinfo?(hkoka)
Unavailable of action button, but on a not-so-common case. Marking this P3 and blocking because of inability for the user to pause or play the video when he reaches this state. 

Assigning to Russ to investigate. 

Thanks
Hema
blocking-b2g: --- → 2.5+
Flags: needinfo?(hkoka)
Priority: -- → P3
Whiteboard: [2.5-aries-test-run-1] → [2.5-aries-test-run-1] [polish]
Assignee: nobody → rnicoletti
Testing with Flame v2.5, I can reproduce the issue. Testing with the latest Flame from master as of Wed Sep 16, I cannot confirm. With the latest from master, when attempting to play the second video, the "Cannot play video - Another app is currently using the video player" error appears.

Regarding the 2.5 behavior, it seems gecko unloads the video from the first view activity because otherwise we would see the "in use" error. However, it may be the video element from the first view activity is not completely unloaded or it's not in a clean state. This theory is based on when pressing the 'play/pause' button when the video from the second view activity is playing, the video app seems to be getting another 'play button' click; each tap of the 'play/pause' button produces two click events, and the state of the video player is toggled from playing to paused but the video does not play.

NI for Sotaro to comment on the 2.5 behavior and the difference in the behavior from master latest.
Flags: needinfo?(sotaro.ikeda.g)
I am going to take a look.
The followings are debugging result on master flame-kk.

- When the problem happens, if I tapped pause button the following functions are called in sequence.
   + HTMLMediaElement::Pause()
   + HTMLMediaElement::Play()
- First video and second video playback uses same HTMLMediaElement instance.
- On second video, handlePlayButtonClick() was called twice for one tap.
  initUI() might register duplicated functions when initUI() is called multiple times.
  initUI() was called before each video activity's playback.
Flags: needinfo?(sotaro.ikeda.g)
Sotaro, from your investigation, do you think this is a gecko issue? Also, how can two videos be played at the same time? I thought the second video wouldn't play because the video codec would be in use playing the first video.
Flags: needinfo?(sotaro.ikeda.g)
I tested on latest master flame-kk. From it, first activity seemed to be reused for second video playback and event listener called twice for one click. Same instance of HTMLMediaElement was used for first video and 2nd video. Fist video was unloaded from the HTMLMediaElement instance before second video's playback and the instance was used to load 2nd video playback.
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Russ Nicoletti [:russn] from comment #15)
> Sotaro, from your investigation, do you think this is a gecko issue?

It do not think it is gecko's issue.

> Also,
> how can two videos be played at the same time? I thought the second video
> wouldn't play because the video codec would be in use playing the first
> video.

When I tested on master flame-kk, activity seemed to be reused. First video was unloaded before 2nd video playback.
(In reply to Russ Nicoletti [:russn] from comment #12)
> I cannot confirm. With the latest from
> master, when attempting to play the second video, the "Cannot play video -
> Another app is currently using the video player" error appears.

By the way, did not see the above error during investigating on 23/Sep master flame-kk.
Assignee: rnicoletti → pdahiya
Synched up with Russ over IRC, assigning myself to help investigate and update with findings. Thanks!
The issue reported here is view inline activity is called twice without first activity closing successfully.

In Video view activity, two actions that trigger done() and release resources is visibility changed and back button.
https://github.com/mozilla-b2g/gaia/blob/master/apps/video/js/view.js#L100
https://github.com/mozilla-b2g/gaia/blob/master/apps/video/js/view.js#L174

Inside done(), we successfully release video resources, close activity and clear local storage key. In issue reported, none of these steps happen when view activity is getting called twice without first activity done called. 

This issue can also be replicated
1) Attach a video file to MMS message
2) Click on attachment to view video.
3) Send a video file via bluetooth and confirm transfer and receive video file via bluetooth.
4) pull down notification tray from the first playing video (viewed from messaging app).
5) Open video received via bluetooth. Video received will start playing but play/pause button doesn't work and has two event listeners registered to it.
Including Russ and David in thread to share findings.

VisibilityChange event is not triggered when the second activity is called and first activity is forced to exit. 
https://github.com/mozilla-b2g/gaia/blob/master/apps/video/js/view.js#L98

Since first activity exits without calling done, inside second activity localStorage.getItem('view-activity-wants-to-use-hardware') is not null, need to figure if we can use it to somehow reset event listeners.

Tim, Setting NI flag to help clarify what happens when an inline activity is called twice. Do we have any way of telling inside activity handler, that activity is about to exit. Thanks!
Flags: needinfo?(timdream)
Flags: needinfo?(rnicoletti)
Flags: needinfo?(dflanagan)
Perhaps when a second activity is invoked and the first is forced to exit, gecko should produce a visibilitychange event. Has there been an explicit decision not to produce a visibilitychange event in that situation?
Flags: needinfo?(rnicoletti)
(In reply to Punam Dahiya [:pdahiya] from comment #21)
> Including Russ and David in thread to share findings.
> 
> VisibilityChange event is not triggered when the second activity is called
> and first activity is forced to exit. 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/video/js/view.js#L98
> 
> Since first activity exits without calling done, inside second activity
> localStorage.getItem('view-activity-wants-to-use-hardware') is not null,
> need to figure if we can use it to somehow reset event listeners.
> 
> Tim, Setting NI flag to help clarify what happens when an inline activity is
> called twice. Do we have any way of telling inside activity handler, that
> activity is about to exit. Thanks!

There is no way to tell the inline activity frame that it is about to exit. Maybe onunload callback might fire; not sure.
Flags: needinfo?(timdream)
Thanks Tim for suggesting to try unload, Debugged and first inline activity window is not getting unloaded and reused for second inline activity when trying to open video received from notification. 

Since we are registering event listeners inside initUI , they get called twice and we have two events registered against UI elements on view window.

https://github.com/mozilla-b2g/gaia/blob/master/apps/video/js/view.js#L50

To fix the issue in this bug, before calling initUI we should check if inline activity is already open and the same window is used to view second inline activity.

As suggested in #comment 20, one way to find that is by checking localStorage.getItem('view-activity-wants-to-use-hardware') is not null. If yes, we should not register event listeners again. I will try this approach and submit patch for review. Thanks!
Comment on attachment 8675802 [details] [review]
[gaia] punamdahiya:Bug1167545 > mozilla-b2g:master

Hi David

Asking for your review as the attached patch touches the code written by you for fixing Bug 1088456.  This checks for localstorage property set to determine, if we are reusing the same activity window for multiple view activity calls (AFAIK possible only through notifications). Please review. Thanks
Flags: needinfo?(dflanagan)
Attachment #8675802 - Flags: review?(dflanagan)
Comment on attachment 8675802 [details] [review]
[gaia] punamdahiya:Bug1167545 > mozilla-b2g:master

Punam,

As we discussed today, I'd prefer a fix here that undoes my too-clever code.  The entire view.js is one big activity handler, and I never thought that it could be called twice. If you move the top-level variables outside of the activity handler and make them global, then you can use one of them (like dom.player) to see if things have already been initialized.

You could even move most of the code out of the activity handler, and then write a small simple activity handler that initialize the UI, but only on the first invocation, and then plays the specified video.

You'd end up with a larger patch, but I think it would be a simpler one, overall, and would improve the code rather than just putting a bandaid on it.
Attachment #8675802 - Flags: review?(dflanagan) → review-
Comment on attachment 8675802 [details] [review]
[gaia] punamdahiya:Bug1167545 > mozilla-b2g:master

Hi David

I have updated patch with your review feedback. Setting review flag again. Thanks!
Attachment #8675802 - Flags: review- → review?(dflanagan)
Comment on attachment 8675802 [details] [review]
[gaia] punamdahiya:Bug1167545 > mozilla-b2g:master

This looks good to me, though I have not tried testing it at all.

Github makes it look like a big patch, but I think that most of the code has only had its indentation level change, right?
Attachment #8675802 - Flags: review?(dflanagan) → review+
(In reply to David Flanagan [:djf] from comment #29)
> Comment on attachment 8675802 [details] [review]
> [gaia] punamdahiya:Bug1167545 > mozilla-b2g:master
> 
> This looks good to me, though I have not tried testing it at all.
I have tested the patch for view activity flows and it looks good.
> 
> Github makes it look like a big patch, but I think that most of the code has
> only had its indentation level change, right?

That's correct, there's no change in Line 77 - Line 119 except its moved inside handleViewActivity.
Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/bcab059827bfee6bf7215068c66473c95d9a60b9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: While there is a video from BT is playing, we play another video file from BT, the play/pause button of the second video is available in step 5&6.
See attachment: verified_Aries KK_v2.5.3gp.
Reproduce rate: 0/10.


Device: Flame KK 2.5 (Pass)
Build ID               20151021065025
Gaia Revision          32d827a70af90a05918f234e5b16b35d5d2a07e8
Gaia Date              2015-10-20 20:57:29
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/473aefe5bd85842eeb142e0cde8e2cd21edbf40b
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151021.104033
Firmware Date          Wed Oct 21 10:40:46 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20151022003511
Gaia Revision          1b902ff26547e2a6c896351a6a73b673f65e19b2
Gaia Date              2015-10-21 14:56:32
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/daa7d98525e859d32a3b3e97101e129a897192a1
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151021.235445
Firmware Date          Wed Oct 21 23:54:53 UTC 2015
Bootloader             s1
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: