889191 - [Video] video player shows spinner for the few events when playing local file.

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title : Using video.js for playing Bluetooth received videos from notification bar.
2. Precondition : Mobile should be connected to internet, bluetooth should be working.
3. Tester's Action : 1. Open video app -> watch a video file
                     2. receives a video file via Bluetooth.
                     3. Try to open newly download video file from notification while watching other video in local video app.
 
4. Detailed Symptom (ENG.) : 
Videos received via bluetooth will be shown in notification bar. If any video is opened from notification, then it uses view.js for playing. 
There are multiple issues found because of playing local video in view.js:
1. video player shows spinner for the few events. This is not intended.
2. When user tries to open Bluetooth received video while watching a local video in video player app, the video playback is not proper. This is because of the limitation of hardware decoder (only one instance of Hardware decoder is supported).

5. Expected : This issue can be avoided if video.js is used to open the video file from notification.

6.Reproducibility: Y
1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: 65e861f5941cab66815a2f79b0bc0a6a0fc8f6fe
9.Personal email id: [e-mail]

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi, 
I am not understand what you mean about: video player shows spinner for the few events. This is not intended.

I found that shows correctly. It shows only when video is under loading.

Comment 2

[Leo]

Hi John,

If video player shows spinner for streaming video, then it is perfect.
But here the spinner is shown while playing the local videos (downloaded via bluetooth). I think this is not correct behavior.

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi, 

Ok. I got it. But the spinner shows only when video is loading. Even if it is a local file, it may need a short time span to load the target frame, and spinner shows and hides immediately. That's why I think it is the correct behavior.

I can need info UX for this part.

BTW, I don't understand what you mean about: the video playback is not proper. I had redone your STR. It just plays the video with view.js instead of video.js, and nothing strange. This kind of behavior is the "view" activity of video app.

Comment 4

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi UX, 

Do we show loading spinner while playing a local file??? The spinner only shows at video element reading frames from file.

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi Leo,

Would mind to tell us more about the video playback is not proper???

I had redone your STR. It just plays the video with view.js instead of video.js, and nothing strange. This kind of behavior is the "view" activity of video app.

Comment 6

[Leo]

Hi John,

Video playback is not proper in the scenatio given below:
When user tries to open Bluetooth received video while watching a local video in video player app, the video playback is not proper. This is because of the limitation of hardware decoder (only one instance of Hardware decoder is supported).

In this scenario already a video is playing from video.js and user is trying to play antoher video from view.js at the same time. But there is only instance of hardware decoder is supported.

It is better to play local videos from video.js to avoid all these issues and confusion.

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

Thanks leo.

I don't think this scenario existing. If you see the code in video.js line 807[1], you will find we will release the video element when it is hide. We did so because what you said, one instance of hardware decoder. If that happened, that should be a bug.

[1] https://github.com/mozilla-b2g/gaia/blob/c8f3474fb73dfe2177ec908f0dcf914d7260248e/apps/video/js/video.js#L807

Comment 8

[Leo]

Hi John,

for your information:
'visibilitychange' event is not firing if we open blutooth dowloaded video from download notification while video is playing in video player app. video player is not minimized when the other video is opened.

Comment 9

[Stephany Wilkes]

Assigning to Casey.

Comment 10

[John Hu [:johnhu][:johu][:醬糊小弟]]

Leo, thanks you for this finding. It's changes of window management staff.

I had discussed with my colleague. This one can be confirmed. The 'visibilitychange' event is not fired when we open another inline 'view' activity of video app. The video element is already used by original video app so that the view activity can't work correctly.

Leo, would you please to open another bug for this one and cc me in?? I will loop my colleague about this part in.

And we should focus on "if we need to separate the playing behavior between local file and remote file with view activity of video app"

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

There's an opened file about visibility-change event change:
See my part(1) at https://bugzilla.mozilla.org/show_bug.cgi?id=887650#c0

Comment 12

[John Hu [:johnhu][:johu][:醬糊小弟]]

Thanks alive. I never found there is already a bug here.

Comment 13

[Leo]

Hi,

Is there any update for this issue?

Comment 14

[John Hu [:johnhu][:johu][:醬糊小弟]]

If kyee doesn't give us a suggestion on "separation of local and remote playing behavior" in this week, I will create a small patch to hide the spinner when playing local file.

Comment 15

[Casey Yee]

In the case of local playback it would make sense not to show the loading indicator unless there is some sort of preload/caching that requires some time before the video shows.

Comment 16

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: prevent spinner to show when playing local file.

Add the code to check if it is local file playing. If yes, do not show the spinner.

Comment 17

[Leo]

Hi John,

I still have the old question for you: 
is it needed to play local file in view.js? 
because view.js is just a replica of video.js and it doesnot have many of the functionalities of video.js.

Please comment.

Comment 18

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi Leo,

I think so. To change to use video.js may need a few works, including merge behaviors of activity, add more checking codes, add flags to know different states, and lots (regression) tests. And I don’t think we can make it before QE4. We had found lots synchronization bugs recently, and may change the implementation as what you said in the future.

Comment 19

[Dale Harvey (:daleharvey)]

Merged in: https://github.com/mozilla-b2g/gaia/commit/147cd38ed0ceda9860ac8b72645e8c9f3ccae149

Thanks John

Comment 20

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 147cd38ed0ceda9860ac8b72645e8c9f3ccae149 to:
v1-train: e218376b4b879cb75708e196fff1e87597c373cb

Comment 21

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: e218376b4b879cb75708e196fff1e87597c373cb