Closed Bug 1005622 Opened 5 years ago Closed 5 years ago

Regression: After MP4 video playback, video can not be played again

Categories

(Core :: Audio/Video, defect)

30 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla32
Tracking Status
firefox29 --- unaffected
firefox30 + fixed
firefox31 + fixed
firefox32 + verified
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
relnote-firefox --- 30+
fennec 30+ ---

People

(Reporter: sfleiter, Assigned: cpearce)

References

()

Details

(Keywords: regression, reproducible)

Attachments

(2 files)

The H.264 video at $URL can not be played again on a Nexus 5 with Android 4.4.2 and Firefox 
https://hg.mozilla.org/mozilla-central/rev/8d591a3f6fea
after it has finished playing.
Neither clicking on the play icon nor seeking backwards allows the video to play again.
Only reloading the page makes that possible.

Bug 793069 described a similar problem, but is in core.
Playing again for this video works on desktop, so this should not be a duplicate.

The video can be played again in chrome for android.
Same device and trunk (Nightly → Beta), I see this issue.
http://people.mozilla.org/~atrain/mobile/tests/media.html 

Used http://people.mozilla.org/~atrain/mobile/tests/big-buck-high.mp4

I'll grab a window shortly.
tracking-fennec: --- → ?
OS: Linux → Android
Summary: Video can not be played again → Regression: After MP4 video playback, video can not be played again
Unfortunately this has been broken since March of this year and the issue currently is on mozilla-beta.

m-c

Last good revision: 41d962d23e81 (2014-03-11)
First bad revision: 44ae8462d6ab (2014-03-12)

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=41d962d23e81&tochange=44ae8462d6ab

m-i

Last good revision: cafc246cc62c
First bad revision: c234a1aeaeab
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cafc246cc62c&tochange=c234a1aeaeab
Blocks: 973408
Flags: needinfo?(cpearce)
Flags: in-testsuite?
Flags: in-moztrap?(fennec)
cpearce@mozilla.com
Tue Mar 11 03:45:27 2014 +0000	c234a1aeaeab	Chris Pearce — Bug 973408 - Merge logic from UpdateIdleState and enqueueing decode tasks, so that we always behave consistently, especially WRT setting readers Idle on B2G. r=kinetik
7ddb43c62d33	Chris Pearce — Bug 973408 - Set MediaDecoderReaders idle when they're not decoding. r=kinetik
a029b4912051	Chris Pearce — Bug 973408 - Decode video and audio in separate media tasks. r=kinetik
67b88c88f821	Chris Pearce — Bug 973408 - Split innards of MediaDecoderStateMachine::DecodeLoop() into sub DecodeAudio/DecodeVideo functions. r=kinetik
8615140a33ca	Chris Pearce — Bug 973408 - Add pop listeners to MediaQueue. r=kinetik
dcaba0ea4352	Chris Pearce — Bug 973408 - Don't block a decode thread while awaiting resources. r=kinetik
5e78ec92e1c1	Chris Pearce — Bug 973408 - Split DecodeThreadRun() into separate decode tasks. r=kinetik
d9dfc05b1be9	Chris Pearce — Bug 973408 - Remove MediaDecoderStateMachine::GetAmpleVideoFrames(), and MediaOMXStateMachine. r=kinetik
b5645f7981c3	Chris Pearce — Bug 973408 - Move MediaDecoderStateMachine::DecodeLoop()'s local variables to class members so the function can be made reentrant in future. r=kinetik
Component: General → Video/Audio
Product: Firefox for Android → Core
Version: Trunk → 30 Branch
Will need to find a device from somewhere before I can debug this.

No other devices are affected as far as we know right?
Flags: needinfo?(cpearce)
Just confirmed this also happens on my Nexus 7 (2012).
jzimbrick - can you have someone check this on Firefox OS 1.4 on an Open C?
Flags: needinfo?(jzimbrick)
(In reply to Stefan Fleiter (:sfleiter) from comment #5)
> Just confirmed this also happens on my Nexus 7 (2012).

I have one of these at home. I'll test on it tomorrow.
This issue reproduces on an Open_C 1.4

1.4 Environmental Variables:
Device: Open_C
BuildID: 20140505123003
Gaia: cacf9065aaedd854245766f968ad7cf6b086ea44
Gecko: 7e9dc74119eb
Version: 30.0
Firmware Version: P821A10-ENG_20140410
Flags: needinfo?(jzimbrick)
Appears to be a popular site in Germany:

http://www.alexa.com/siteinfo/m.focus.de

We probably want to block on this for B2G as well.
blocking-b2g: --- → 1.4?
blocking-b2g: 1.4? → 1.4+
(In reply to Chris Pearce (:cpearce) from comment #4)
> Will need to find a device from somewhere before I can debug this.
> 
> No other devices are affected as far as we know right?

Also broken on my new Galaxy S5.
Looks like a pretty bad regression that we should try to never ship, it's already in Beta which is unfortunate. Tracking and assigning to cpearce while he investigates with local device.
Assignee: nobody → cpearce
We may also want to put this as a known issue on our beta/aurora release notes in the meantime if we're starting to see it affecting a lot of popular devices.
relnote-firefox: --- → ?
(In reply to Harvey from comment #8)
> This issue reproduces on an Open_C 1.4
> 
> 1.4 Environmental Variables:
> Device: Open_C
> BuildID: 20140505123003
> Gaia: cacf9065aaedd854245766f968ad7cf6b086ea44
> Gecko: 7e9dc74119eb
> Version: 30.0
> Firmware Version: P821A10-ENG_20140410

What device is "Open_C 1.4"?
(In reply to Chris Pearce (:cpearce) from comment #13)
> (In reply to Harvey from comment #8)
> > This issue reproduces on an Open_C 1.4
> > 
> > 1.4 Environmental Variables:
> > Device: Open_C
> > BuildID: 20140505123003
> > Gaia: cacf9065aaedd854245766f968ad7cf6b086ea44
> > Gecko: 7e9dc74119eb
> > Version: 30.0
> > Firmware Version: P821A10-ENG_20140410
> 
> What device is "Open_C 1.4"?

http://www.ebay.com/itm/PRE-SALE-ZTE-OPEN-C-dual-core-Latest-Firefox-OS-3G-Unlocked-Smartphone-/351033890463
Attached patch PatchSplinter Review
The problem is that when we play to the end of file, we call Finish() on the audio and video queues. If we subsequently seek, MediaPluginReader::Seek() doesn't call MediaQueue::Reset() in order to reset the IsFinished() flag, so after the seek we immediately assume we've reached EOS, and stop playing.

MediaPluginReader::Seek() should be calling MediaQueue::Reset() rather than MediaQueue::Erase(). I have another patch that removes Erase() actually, I should really land that sometime, so that we avoid issues like this.

I don't know why this failure is visible on B2G, as B2G uses a different code path here than Android Firefox/Fennec.

MediaOmxReader::Seek() (the B2G code) calls MediaDecoderReader::ResetDecode(), which resets the MediaQueues, so I don't know how the same behaviour could possibly be happening on B2G. I'll need a B2G devices that reproduces this bug to fix it. None of my B2G devices reproduce this bug on tip gecko/B2G.

I also spotted a compile warning about size mismatch, printing 64 bit int with %ld instead of %lld.
Attachment #8418500 - Flags: review?(edwin)
(In reply to Chris Pearce (:cpearce) from comment #15)

> I also spotted a compile warning about size mismatch, printing 64 bit int
> with %ld instead of %lld.

There are hundreds of such warnings in netwerk/ and some in content/media...
Can somebody s(In reply to Jason Smith [:jsmith] from comment #14)
> (In reply to Chris Pearce (:cpearce) from comment #13)
> > (In reply to Harvey from comment #8)
> > > This issue reproduces on an Open_C 1.4
> > > 
> > > 1.4 Environmental Variables:
> > > Device: Open_C
> > > BuildID: 20140505123003
> > > Gaia: cacf9065aaedd854245766f968ad7cf6b086ea44
> > > Gecko: 7e9dc74119eb

This is not a version of Gecko in mozilla-central AFAICT. What changeset is this? Is this a version of gecko build by ZTE that has more custom hacks that break the bundled video decoder?

The ZTE bug should be a separate bug, the Firefox Android and B2G video code paths are different.
Fixed in 32:
Also removed the call to Erase() in RawReader.cpp.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fb3807811b7
(In reply to Chris Pearce (:cpearce) from comment #17)
> The ZTE bug should be a separate bug, the Firefox Android and B2G video code
> paths are different.

Also, because the code paths on Fennec and B2G are different, we'll need to re-verify the regression range for B2G as well. The B2G bug could be something unrelated, and not a regression from bug 973408.
Note: the fix here *may* affect the orange profile of media mochitests on android. The sheriffs disabled a couple yesterday on android for orangeness.
Added in the release notes for 30 aurora and 31 beta with the wording "After MP4 video playback, video can not be played again"
https://hg.mozilla.org/mozilla-central/rev/5fb3807811b7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
I'm really confused here. So this patch is blocking-b2g:1.4+ even though comment 17 suggests that the fix here won't help B2G?

Also, I assume this does need Aurora/Beta nominations?
Flags: needinfo?(cpearce)
Flags: needinfo?(bbajaj)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #23)
> I'm really confused here. So this patch is blocking-b2g:1.4+ even though
> comment 17 suggests that the fix here won't help B2G?

This fix will not help B2G. B2G should be fixed in a separate bug. If I'm to work on B2G, we'll need an Open C device sent to Auckland.

FWIW I asked Blake Wu in Taipei to see if this bug reproduces on his Open C, and it does not. So I'm completely unsure this is an actual bug on B2G.


> Also, I assume this does need Aurora/Beta nominations?

Yes, absolutely. I'd like to see this verified fixed on Fennec before requesting uplift however.
Flags: needinfo?(cpearce)
Seems this needs reconsideration on the 1.4+ then, thanks :)
blocking-b2g: 1.4+ → 1.4?
Ok - let me get someone double check exactly what behavior was seen on 1.4 on Open C.

jharvey - can you clarify exactly what you saw on Open C on 1.4?
blocking-b2g: 1.4? → ---
Flags: needinfo?(jharvey)
I retested this issue myself on an Open C device running the latest 1.4 build.

I did NOT see this issue reproduce using the video provided in this bug's URL field, or with the video listed in Comment 1.

The videos both played to the end without issue, and then the user is able to restart the video from the beginning by tapping the play button. The tracking bar also behaved as expected during both videos, allowing the user to navigate forward or backward to different parts of the video.

Environmental Variables:
Device: Open C
BuildID: 20140507000202
Gaia: bac831d2ebc567f0939e41fbd8a4c15ef183b954
Gecko: 8040ccd0d4b1
Version: 30.0
Base Image: P821A10-ENG_20140410

We have experienced network issues in the past that may have produced a false positive at the time that comment 8 was submitted.
Flags: needinfo?(jharvey)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #23)
> I'm really confused here. So this patch is blocking-b2g:1.4+ even though
> comment 17 suggests that the fix here won't help B2G?
> 
> Also, I assume this does need Aurora/Beta nominations?

When blocking for 1.4 we assumed it was a common platform issue affecting FxOS, may be its a separate issue, looks like QA is investigating this already.
Flags: needinfo?(bbajaj)
With these patches in mozilla-central (Nightly 05/07), on attempt to verify what I am seeing is the inability to re-play the video [1] again after the video clip is over for a period of time (several seconds) due to what seemingly looks like no 'stop' event happening when the duration of the video has played. After several seconds, then can one re-play the video.

[1] http://people.mozilla.org/~atrain/mobile/tests/big-buck-high.mp4

There's also a regression here in that the duration counter is seemingly *adding* additional time on attempt's to replay.

The initial video is 0:34 seconds, on attempt to replay the duration counter becomes 1:08, after the end of the video clip is a blank white frame and there is no stop event (the pause button remains visible on video tap).

Since this is causing more regressions, I am re-opening the bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I was also verifying this issue, and I can confirm the same behaviour Aaron described in the previous comment. Tested on https://people.mozilla.org/~cpeterson/videos/spaceballoon.800x450.mp4 , device: Alcatel One Touch 8008D(Android 4.1.2)
tracking-fennec: ? → 30+
Thanks Aaron and Mihai.

Aaron, I cannot reproduce the duration doubling issue you describe in comment 29 on my Nexus 7 in Nightly 05/08. Can you please post explicit steps to reproduce? Or record a video with another device demonstrating how to reproduce the bug?

I do see an issue that sometimes when playback reaches the end of video the pause button doesn't change to a play button, implying that end of stream wasn't detected properly. I think this is the same as the other behaviour you describe.
Flags: needinfo?(aaron.train)
Samsung Galaxy S5 (Android 4.4.2)
Nightly  (05/08)

Video: https://www.youtube.com/watch?v=EKgIXFNSupA

Steps to Reproduce

  * Visit http://people.mozilla.org/~atrain/mobile/tests/big-buck-high.mp4 and play the entirety of the clip
  
  * Tap the screen after the clip ends, see the video controls appear and the duration mention 0:34
  
  * Wait until the video controls re-appear on the screen (when what seemingly looks like a proper stop event) and see the duration doubled to 1:08
  
  * Tap the play button again, the clip will play and end at the original duration 0:34 despite having a timeline of 1:08. The last frame shown is white and the scrubber stops at 0:34. On tapping Pause, and Play again, the scrubber jumps to the 1:08 mark
Flags: needinfo?(aaron.train)
The problem here is that the last frame is marked with a duration that its end time. So instead of finishing at time $end_time, the video stream finishes at 2*$end_time, since the last frame is the length of the entire media. This is a regression from Bug 930829. I fill file another bug for this, since it's a separate issue. We'll want to uplift it too.
Filed: Bug 1008785 - End frame of video has duration of entire video
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Comment on attachment 8418500 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 973408
User impact if declined: MP4 videos on android won't be re-playable (unless user reloads the page/video).
Testing completed (on m-c, etc.): Been on Nightly for a few days. Local testing.
Risk to taking this patch (and alternatives if risky): Low.
String or IDL/UUID changes made by this patch: None.
Attachment #8418500 - Flags: approval-mozilla-beta?
Attachment #8418500 - Flags: approval-mozilla-aurora?
Attachment #8418500 - Flags: approval-mozilla-beta?
Attachment #8418500 - Flags: approval-mozilla-beta+
Attachment #8418500 - Flags: approval-mozilla-aurora?
Attachment #8418500 - Flags: approval-mozilla-aurora+
Attached patch Beta PatchSplinter Review
Rebased ontop of beta. For some reason I get an error when I try to push to beta "abort: HTTP Error 403: ssl required", so I'll need help landing this.
Attachment #8421260 - Flags: review+
Attachment #8421260 - Flags: checkin?(ryanvm)
Flags: needinfo?(cpearce)
Attachment #8421260 - Flags: checkin?(ryanvm)
d'oh, incorrect default-push, push works now, thanks RyanVM!
https://hg.mozilla.org/releases/mozilla-beta/rev/63b001b6f985
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #39)
> https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/63b001b6f985

This is unnecessary. This patch changes code that is not compiled in b2g, it's fennec-only code.

This bug should be marked as "status-b2g-v1.4: unaffected" and "status-b2g-v2.0: unaffected", and this changeset can probably be backed out of mozilla-b2g30_v1_4 since it does nothing for b2g.
Note the cset hash. beta is merged to b2g30 - i.e. anything that lands on beta lands on b2g30. Relax :)
Status: RESOLVED → VERIFIED
Thanks for fixing this.
This also fixed a (minor) hitch where after the advertisement played at $URL above there was a pause before the read video started.
I now assume the pause was caused by the fact that the last frame (of the advertisement video) had a play time of the whole video.
The pause might have been as long as the real duration of the advertisement.
That sounds like bug 1008785 which should be fixed in Nightly builds. Could you retest in a Nightly build?
That is what I wanted to say. :-)
On nightly on my Nexus 5 this is fixed, sorry for commenting on the wrong bug.
Thanks a lot for your work!
You need to log in before you can comment on or make changes to this bug.