Closed Bug 1147090 Opened 5 years ago Closed 5 years ago

Flame: Coming back from lockscreen, video player does not show frame at 00:00 frame

Categories

(Core :: Audio/Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

VERIFIED FIXED
2.2 S9 (3apr)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- verified
b2g-master --- unaffected

People

(Reporter: njpark, Assigned: sotaro)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

STR:
Open video player, select video, pause, then rewind to 00:00
Press power button to go to lockscreen
Press power again and resume video app

Expected:
Video screen is shown
Actual:
Video screen is blank.

This does not reproduce in 3.0, and only reproduces in 2.2

Version Info:
Gaia-Rev        014d38f7ad3912b8b33cb08ce7535a5dc5aced59
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7a9f2a248e57
Build-ID        20150324002504
Version         37.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150324.041652
FW-Date         Tue Mar 24 04:17:03 EDT 2015
Bootloader      L1TC000118D0

Sotaro thinks Bug 1062134 might be causing this regression
Keywords: regression
See Also: → 1062134
Reason for blocking nomination:
Regression, and bad UX behavior that can be encountered in common use scenarios.
blocking-b2g: --- → 2.2?
I am going to investigate.
Assignee: nobody → sotaro.ikeda.g
On master, the problem does not happen. It seems to be fixed by Bug 1135170. I do not think that it could be uplifted to b2g-v2.2. The change and dependent bugs are a lot.
MediaDecoderStateMachine does not load first video frame when it is resumed from dormant state since Bug 1062134 fix. MediaDecoderStateMachine expects video frame update by playback or seek.

In this bug's STR, seek should trigger video frame update. But until Bug 1135170 fix, seeks does not update video frame by seek if seeking time is same. The related code is in MediaDecoderStateMachine::DecodeSeek().

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/file/cb4856ecd781/dom/media/MediaDecoderStateMachine.cpp#l2463
Component: Gaia::Video → Video/Audio
Product: Firefox OS → Core
From comment 4, we could say that this bug is a regression of Bug 1062134.
The patch fixed the problem on v2.2 flame.
Attachment #8582700 - Flags: review?(cpearce)
Comment on attachment 8582700 [details] [diff] [review]
patch for b2g v2.2 - Change seek skip check

Review of attachment 8582700 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/MediaDecoderStateMachine.cpp
@@ +2485,5 @@
>  
>    mDecodeToSeekTarget = false;
>  
> +  if (!currentTimeChanged &&
> +      !(HasVideo() && (VideoQueue().GetSize() == 0) && !VideoQueue().IsFinished())) {

Does it make sense for this to be:

if (!currentTimeChanged &&
    (!HasVideo() || (VideoQueue().GetSize() == 0 && !VideoQueue().IsFinished())))

Then we still take the !currentTimeChanged when seeking in an audio-only file.
Attachment #8582700 - Flags: review?(cpearce) → review+
(In reply to Chris Pearce (:cpearce) from comment #7)
> Comment on attachment 8582700 [details] [diff] [review]
> patch for b2g v2.2 - Change seek skip check
> 
> Review of attachment 8582700 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/MediaDecoderStateMachine.cpp
> @@ +2485,5 @@
> >  
> >    mDecodeToSeekTarget = false;
> >  
> > +  if (!currentTimeChanged &&
> > +      !(HasVideo() && (VideoQueue().GetSize() == 0) && !VideoQueue().IsFinished())) {
> 
> Does it make sense for this to be:
> 
> if (!currentTimeChanged &&
>     (!HasVideo() || (VideoQueue().GetSize() == 0 &&
> !VideoQueue().IsFinished())))
> 
> Then we still take the !currentTimeChanged when seeking in an audio-only
> file.

I assume you mean the following. It is a logical equivalence to attachment 8582700 [details] [diff] [review]. Yeah, it makes the intent more clear :-)

if (!currentTimeChanged &&
    (!HasVideo() || !(VideoQueue().GetSize() == 0 && !VideoQueue().IsFinished())))
Apply the comment. Carry "r=cpearce".
Attachment #8582700 - Attachment is obsolete: true
Attachment #8583069 - Flags: review+
Blocks: 1062134
Comment on attachment 8583069 [details] [diff] [review]
patch for b2g v2.2 - Change seek skip check

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 1062134
User impact if declined: User might not see the video frame when the video app goes back to foreground.
Testing completed: locally tested.
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #8583069 - Flags: approval-mozilla-b2g37?
blocking-b2g: 2.2? → 2.2+
Keywords: verifyme
Attachment #8583069 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Keywords: checkin-needed
Whiteboard: [only b2g-v2.2]
The patch is tested on 2.2, and it looks good.

Tested Gecko commit: efdb447485a5d67b783ba890520787a1c0e6638f
Thanks!
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/f1f6abcbee5c
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [only b2g-v2.2]
Target Milestone: --- → 2.2 S9 (3apr)
This bug has been verified as "pass" on latest Nightly build of Flame v2.2 and Nexus 5 v2.2 by the STR in Comment 0.

Actual results: Coming back from lockscreen, video screen/frame is shown normally at 00:00 frame.
See attachment: verified_Flame_v2.2.3gp
Reproduce rate: 0/10


Device: Flame v2.2 (Verified) 
Build ID               20150706002507
Gaia Revision          ea11f422b687a982f0a961c9aea7858066561707
Gaia Date              2015-07-02 23:37:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c0214b4c1ea0
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150706.035706
Firmware Date          Mon Jul  6 03:57:18 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 v2.2 (Verified) 
Build ID               20150706002507
Gaia Revision          ea11f422b687a982f0a961c9aea7858066561707
Gaia Date              2015-07-02 23:37:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c0214b4c1ea0
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150706.041056
Firmware Date          Mon Jul  6 04:11:15 EDT 2015
Bootloader             HHZ12f
Status: RESOLVED → VERIFIED
Keywords: verifyme
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.