User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36 Steps to reproduce: Because of security problem, I can not attach video file. So, I have wrote example step. [Example step] If (A)video file's I-Frame is on 0, 10, 20, 30 sec and duration is 30 sec, you can reproduce problem like below step. 1.Copy attached (A) file to your device. 2.Since (A) video file's I-Frame is on 10 sec and 20 sec, if you seek to 15 sec, player will be started at 10 sec. 3.But Video player position is 15 sec and not changed until play position is over 15 sec. Actual results: When I checked seek operation in video player, I have found problem. Currently, Gecko is using I-Frame seek to operate seek function. But, when player operates I-Frame seek, Video player's position was not updated like above step. When player seek to I-Frame, Video player's position should be changed to I-Frame position.(ex: 10 sec) As a result of debugging, because of below code, position was not changed. When I remove the "if" condition of Code.1 or Code.2, (1. "if (newCurrentTime != mediaTime)" OR 2. "if (clock_time > GetMediaTime())") Video player's position is updated. 1. [Code in MediaDecoderStateMachine::DecodeSeek] ...... if (newCurrentTime != mediaTime) { UpdatePlaybackPositionInternal(newCurrentTime); if (mDecoder->GetDecodedStream()) { SetSyncPointForMediaStream(); } } ...... 2. [Code in MediaDecoderStateMachine::AdvanceFrame()] ...... if (clock_time > GetMediaTime()) { // Only update the playback position if the clock time is greater // than the previous playback position. The audio clock can // sometimes report a time less than its previously reported in // some situations, and we need to gracefully handle that. UpdatePlaybackPosition(clock_time); } ...... Please check this problem. Thanks.

Jaemin - Can you email me offline (jsmith@mozilla.com) with the video test file? My team would like to see if we can reproduce the bug.

Because of security problem, I can not share video file. But this issue can be detected using other file.(Sheets.mp4) Sheets.mp4 file is in "gaia/apps/ftu/style/images/tutorial/Sheets.mp4" (To check other bug, I had already share this file in Bug 1050667) [Sheets.mp4 information] Duration : 8 sec I-Frame: 0, 5 sec If you try to play "Sheets.mp4"file and seek to 4 sec, you can find problem. Thanks.

I do not think we support the type of media file(vp8/vp9) referrred in 1050667, so this is a known limitation. I am NI eric chou to comment here as well to see if that would be resolved in 2.1

(In reply to bhavana bajaj [:bajaj] from comment #3) > I do not think we support the type of media file(vp8/vp9) referrred in > 1050667, so this is a known limitation. > > I am NI eric chou to comment here as well to see if that would be resolved > in 2.1 "Sheets.mp4" file is not vp8/vp9 file. Codec of this file(Sheets.mp4) is AVC. Thanks.

I am going to investigate this problem.

Cannot reproduce this problem on my Flame (OS:2.0.0.0-prerelease, Platform version:32.0a2, build identified: 20140721000201) with testing content, Sheets.mp4.

(In reply to Blake Wu [:bwu][:blakewu] from comment #6) > Cannot reproduce this problem on my Flame (OS:2.0.0.0-prerelease, Platform > version:32.0a2, build identified: 20140721000201) with testing content, > Sheets.mp4. When you seek to 4 sec, is position of progress bar changed to 0 sec? Since "sheets.mp4"'s I-Frame is 0 sec and 5 sec, it should be changed to 0 sec. In my device(OS 2.0), when I seek to 4 sec, position of progress bar is 4 sec and not updated to 0 sec. In this time, player is showing video frame of 0 sec. And if video time stamp is over 4 sec, position of progress bar is updated. Please check again. Thanks.

Were you able to reproduce on a flame device?

(In reply to Jaemin Song from comment #7) > (In reply to Blake Wu [:bwu][:blakewu] from comment #6) > > Cannot reproduce this problem on my Flame (OS:2.0.0.0-prerelease, Platform > > version:32.0a2, build identified: 20140721000201) with testing content, > > Sheets.mp4. > > When you seek to 4 sec, > is position of progress bar changed to 0 sec? > Since "sheets.mp4"'s I-Frame is 0 sec and 5 sec, > it should be changed to 0 sec. Yes. My *Flame* will jump to 4 0 sec, but my *Madai* plays from 4 sec. So the behavior is different. May need to further check if APP use fastSeek API for seeking.

(In reply to Wayne Chang [:wchang] from comment #8) > Were you able to reproduce on a flame device? Because I do not have a flame device, I can not reproduce on a flame device.

(In reply to Blake Wu [:bwu][:blakewu] from comment #9) > Yes. My *Flame* will jump to 4 0 sec, but my *Madai* plays from 4 sec. > So the behavior is different. > May need to further check if APP use fastSeek API for seeking. I have check whether APP use fastSeek API for seeking. As a result of check, APP use fastSeek API for seeking. And, I have confirmed that seekType is "PrevSyncPoint" on "MADAI". When I seek to 4 sec, my *Madai* also plays from 4 sec. But actually video time stamp is 0 sec. When I seek to 4 sec and video's time stamp is changed to 0 sec,(PrevSyncPoint) I have compared with time stamp and position of Video Player like below. ----------------------------------------------- sec |0 1 2 3 4 5 6 7 8 ----------------------------------------------- (Decoder) | video frame |0 1 2 3 4 5 6 7 8 time stamp | ----------------------------------------------- (APP) | Video Player |4 4 4 4 4 5 6 7 8 progress bar position | ----------------------------------------------- I want to know whether your *Madai* use fastSeek API. Thanks.

It looks like my Madai uses fastSeek since for seeking to 4 sec, it starts to play from 0-sec frame but the progress bar stays at 4 sec until the playback reaches 4-sec. So the only difference between Flame and Madia is Flame's progress bar will be updated with seeking time which is a correct behavior, but Madai doesn't.

(In reply to Blake Wu [:bwu][:blakewu] from comment #12) > It looks like my Madai uses fastSeek since for seeking to 4 sec, it starts > to play from 0-sec frame but the progress bar stays at 4 sec until the > playback reaches 4-sec. > > So the only difference between Flame and Madia is Flame's progress bar will > be updated with seeking time which is a correct behavior, but Madai doesn't. Yes. So, I had checked code for debugging like comment 0. Since above code in comment 0, "mCurrentFrameTime is not updated. (Progress bar position is updated from "mCurrentFrameTime".) It should be checked why "mCurrentFrameTime" is not updated. Please confirm it. Thanks.

Codes in Gecko on both Flame and Madai should be the same. I will confirm it once I get the source codes of Madai.

Jaemin, I am 2.0-'ing this because it does not reproduce on flame, and we cannot debug unless it can be seen on a flame device.

Unassign myself since cannot get the source codes and as comment 15 said.

(In reply to Wayne Chang [:wchang] from comment #15) > Jaemin, > > I am 2.0-'ing this because it does not reproduce on flame, and we cannot > debug unless it can be seen on a flame device. I have checked also this issue on Flame device. Problem was not occurred. It should be checked what is the difference between Flame and Madai. I'll checked it more. Thanks.

When I operate seek function on Madai device, video app was calling "fastSeek" function 3~4 times. Because of this, "mCurrentFrameTime" in MediaDecoderStateMachine(gecko) was updated again to user's seek position. It should be checked why video player calls "fastSeek" function several times. (On Flame, video player is called "fastSeek" function just one times.) I'll request review to engineer of video app.

Only occurring on vendor device.

Change to Vendcom per Comment#18, Comment#19

Hi guys, Sireesha pinged me at irc and asked me to see this one and especially comment 18: (my replying) We do call fastSeek() multiple times[1]. That's expected behavior and we moves the time and player head at the same time based on the movements of user's finger. Once touchmove event fired brings one function call. Video app doesn't know if the user's finger is touched at I-frame, and fastSeek() will move to I-frame for video app. If video element is still in seeking mode, the video element should be able to handle the multiple function call. And Video app uses player.seeking[2] to prevent unstable UI update while it is seeking. That's as expected no matter in calling fastSeek() or updating "currentTime". [1] https://github.com/mozilla-b2g/gaia/blob/v2.0/apps/video/js/video.js#L1234-L1245 [2] https://github.com/mozilla-b2g/gaia/blob/v2.0/apps/video/js/video.js#L798-L804

Blake, Sireesha also told me that Simulator can reproduce it. I can confirm that with "WebM" file in simulator. You can try it or come to my place to see it. cc Russ(another peer of video app) to know this.

I cannot reproduce it in firefox nightly. As per comment 14, the gecko and gaia codes in flame and madai should be the same and this issue may be gonk issue. Currently status: Flame: unable to reproduce with v2.0 branch and master Madai: able to reproduce with v2.0 Firefox Simulator 2.0: able to reproduce with WebM file Firefox Nightly: unable to reproduce with WebM file BTW, there is a bug 1059661 which may be related to this one. I can reproduce it in Firefox Simulator 2.0 ,but not in Firefox Nightly and flame.

(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #23) > I cannot reproduce it in firefox nightly. > > As per comment 14, the gecko and gaia codes in flame and madai should be the > same and this issue may be gonk issue. > > Currently status: > Flame: unable to reproduce with v2.0 branch and master > Madai: able to reproduce with v2.0 > Firefox Simulator 2.0: able to reproduce with WebM file > Firefox Nightly: unable to reproduce with WebM file > > > BTW, there is a bug 1059661 which may be related to this one. I can > reproduce it in Firefox Simulator 2.0 ,but not in Firefox Nightly and flame. This issue is not related to bug 1059661. bug 1059661 is QCT Extractor problem. This case's video file is using native MPEG4 Extractor. Thanks.

I can reproduce this issue on Flame(v2.0) [Steps to reproduce] 1. Play attached video file. 2. While playing video file, operate "drag seek" to 18 sec.(OR 28 sec.. etc) (Please operate "drag seek" to where is not the I-Frame.) 3. Position is not changed.(Problem is occurred.) Reproducible rate: 50% Dear Blake, Please confirm this issue on Flame. Thanks.

(In reply to Jaemin Song from comment #25) > I can reproduce this issue on Flame(v2.0) > > [Steps to reproduce] > 1. Play attached video file. > 2. While playing video file, operate "drag seek" to 18 sec.(OR 28 sec.. etc) > (Please operate "drag seek" to where is not the I-Frame.) > 3. Position is not changed.(Problem is occurred.) > > Reproducible rate: 50% > > Dear Blake, > Please confirm this issue on Flame. > > Thanks. Hi Jaemin - Still we probably need the video file you mentioned to reproduce, is it possible you can reproduce this issue with some other video clip without copy-right protection? Thanks Vance

Yes. This issue is occurred using "Sheet.mp4" file. (File link: https://bugzilla.mozilla.org/attachment.cgi?id=8475748) If you operate "drag seek" to 4 sec. You can find problem. Thanks.

I can repro this problem on my Flame V2.0, but cannot on V2.1.

I am going to check if there is some gecko's patch on V2.1 which fixed this problem. John, could you help check the gaia side? Thanks!

Chens will check this bug. Thanks.

Thanks! :) I found this is more related to gecko's side.

Thanks Blake, I also found something that maybe related to this bug in the observation. Refer to the log in attachment 8491261 [details] and screen shot in attachment 8491263 [details], under some circumstances player time will become 0 when seeking the video, maybe could also help on this issue.

chens, One question. This player time is reported by gecko, right?

yep you are right, player time is retrieved from video element and it's from gecko.

From gecko side, This problem is a timing issue which happens in a quick series of seeking, so dragging seek can easily repro this problem. If use seeks to 3-second (fastseek will seek to the previous I-frame, lets say 83msec) and the current decoded frame is still 83msec due to the previous seek to I-frame, mCurrentFrameTime will not be updated in DecodeSeek() @line#2033 (shown below). Another resaon to cause that is mediaTime is obtianed @line#1919, but it is not updated when being checked @line#2032, since this value may be changed @line#1926. So newCurrentTime = mediaTime = 83msec, UpdatePlaybackPositionInternal() will not be called @line#2033. 1898 void MediaDecoderStateMachine::DecodeSeek() 1899 { : : bool currentTimeChanged = false; 1919 const int64_t mediaTime = GetMediaTime(); 1920 if (mediaTime != seekTime) { 1921 currentTimeChanged = true; 1922 // Stop playback now to ensure that while we're outside the monitor 1923 // dispatching SeekingStarted, playback doesn't advance and mess with 1924 // mCurrentFrameTime that we've setting to seekTime here. 1925 StopPlayback(); 1926 UpdatePlaybackPositionInternal(seekTime); 1927 } : : 2032 if (newCurrentTime != mediaTime) { 2033 UpdatePlaybackPositionInternal(newCurrentTime); 2034 if (mDecoder->GetDecodedStream()) { 2035 SetSyncPointForMediaStream(); 2036 } 2037 } 2038

BTW, the analysis is based on the test content, "Sheet.mp4".

According to comment 37, make mediaTime update in time to make UpdatePlaybackPositionInternal() will be called. This patch only works for V2.0 since the seek behavior of V2.1 is different and doesn't have this problem.

IMO，this is not so serious to check in codes to fix. The reasons are 1. This problem occurs when user quickly does seeking (repro rate: ~20%) or drag seeking (~40%). When the problem happens, video still can be played well only UI doesn't reflect the correct time for a while. 2. AFAIK, V2.0 code is in cc stage. 3. V2.1 doesn't have this problem. This is a v2.0 specific problem. Wayne and Vance, How do you think?

I agree that this is probably not needed to land in 2.0 given the circumstances, and also that 2.1 does not have this problem. However, let Vance check with our partner and confirm. Please note if there's any code landing needed we should open a new public bug to describe. (I believe the patches here also are not needed by master anymore?)

(In reply to Blake Wu [:bwu][:blakewu] from comment #40) > IMO，this is not so serious to check in codes to fix. > The reasons are > 1. This problem occurs when user quickly does seeking (repro rate: ~20%) or > drag seeking (~40%). When the problem happens, video still can be played > well only UI doesn't reflect the correct time for a while. > 2. AFAIK, V2.0 code is in cc stage. > 3. V2.1 doesn't have this problem. This is a v2.0 specific problem. In MADAI, because video player calls "fastseek" multiple times whenever user operates seek function, this problem is always occurred. (Please refer comment 18 and comment 21) So, this patch is needed. Thanks.

Agree with Jaemin, For Madai, it is a common user scenario for end users to use Madai to watch various devices. So Black, could you help to provide the patch for this issue? Thanks for your help Vance

Comment on attachment 8491310 [details] [diff] [review] Bug-1053051-update-mediaTime-before-checked-in-DecodeSeek.patch cpearce, Could you help review this patch? Thanks!

Comment on attachment 8491310 [details] [diff] [review] Bug-1053051-update-mediaTime-before-checked-in-DecodeSeek.patch Review of attachment 8491310 [details] [diff] [review]: ----------------------------------------------------------------- JW, please review this.

Comment on attachment 8491310 [details] [diff] [review] Bug-1053051-update-mediaTime-before-checked-in-DecodeSeek.patch Review of attachment 8491310 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/MediaDecoderStateMachine.cpp @@ +2033,1 @@ > if (newCurrentTime != mediaTime) { nit: say |newCurrentTime != GetMediaTime()|, don't change the meaning and constness of |mediaTime|.

Jaemin, If you don't have other concern, I will make this bug public since this bug is common on V2.0.

(In reply to Blake Wu [:bwu][:blakewu] from comment #47) > Jaemin, > If you don't have other concern, I will make this bug public since this bug > is common on V2.0. OK. Please make this bug public.

[Blocking Requested - why for this release]: According to comment 42 and comment 43.

Triage to set it to 2.0+.

https://tbpl.mozilla.org/?tree=Try&rev=2190705cf024 Ryan, May I have your help to check the orange and red ones? After checking the logs, I don't think my patch should not cause those test case failed or make build break :(

(In reply to Blake Wu [:bwu][:blakewu] from comment #51) > https://tbpl.mozilla.org/?tree=Try&rev=2190705cf024 > Ryan, > May I have your help to check the orange and red ones? > After checking the logs, I don't think my patch should not cause those test > case failed or make build break :( None of those tests run on b2g32 normally (see https://tbpl.mozilla.org/?tree=Mozilla-B2g32-v2.0), so those failures aren't surprising.

Carry reviewer, jwwang.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #52) > (In reply to Blake Wu [:bwu][:blakewu] from comment #51) > > https://tbpl.mozilla.org/?tree=Try&rev=2190705cf024 > > Ryan, > > May I have your help to check the orange and red ones? > > After checking the logs, I don't think my patch should not cause those test > > case failed or make build break :( > > None of those tests run on b2g32 normally (see > https://tbpl.mozilla.org/?tree=Mozilla-B2g32-v2.0), so those failures aren't > surprising. Cool! Thanks!

TPBL results (https://tbpl.mozilla.org/?tree=Try&rev=2190705cf024) looks good and reasonable :)

Comment on attachment 8494933 [details] [diff] [review] Bug-1053051-Read-MediaTime-when-checked-in-DecodeSeek-Final.patch 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 #): NA User impact if declined: Video progress bar sometimes may not move after seeking Testing completed: Done. Risk to taking this patch (and alternatives if risky): Less risk. String or UUID changes made by this patch:

My understanding is that this bug only affects FxOS v2.0. Assuming that's the case, this can't land until it has approval. Please wait to request checkin until it does.

Comment on attachment 8494933 [details] [diff] [review] Bug-1053051-Read-MediaTime-when-checked-in-DecodeSeek-Final.patch Jaemin, Can you please try the latest 2.0 and confirm the patch fixes your issue?

(In reply to bhavana bajaj [:bajaj] from comment #58) > Comment on attachment 8494933 [details] [diff] [review] > Bug-1053051-Read-MediaTime-when-checked-in-DecodeSeek-Final.patch > > Jaemin, > > Can you please try the latest 2.0 and confirm the patch fixes your issue? I have checked this patch. After applying patch, issue is fixed. Thanks.

Got approval-mozilla-b2g32+ and the patch is verified.

Backed out for desktop test failures. https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f19f4e07010a https://treeherder.mozilla.org/ui/logviewer.html#?job_id=34072&repo=mozilla-b2g32_v2_0

Dear Blake, Please check this failures. Thanks.

(In reply to Jaemin Song from comment #63) > Dear Blake, > > Please check this failures. > > Thanks. Sure. I'm checking.

Make timestamp are up to date when seek.

Carry reviewer, jwwang.

The test results look good @https://tbpl.mozilla.org/?tree=Try&rev=60034b77e886 compared to mozilla-b2g32_v2_0 (https://treeherder.mozilla.org/ui/index.html#/jobs?repo=mozilla-b2g32_v2_0)