1322799 - Remove AccurateSeekTask

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Comment 1

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 1 - un-encapsulating SeekTask;

Review commit: https://reviewboard.mozilla.org/r/98018/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98018/

Comment 2

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 2 - let AccurateSeekingState request demuxer seeking;

Review commit: https://reviewboard.mozilla.org/r/98020/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98020/

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 3 - move AccurateSeekTask::OnSeek{Resoved,Rejected}();

Review commit: https://reviewboard.mozilla.org/r/98022/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98022/

Comment 4

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 4 - move AccurateSeekTask::Request{Audio,Video}Data();

Review commit: https://reviewboard.mozilla.org/r/98024/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98024/

Comment 5

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 5 - move AccurateSeekTask::Handle{Audio,Video,Not}{Decoded,Waited}(); }

Review commit: https://reviewboard.mozilla.org/r/98026/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98026/

Comment 6

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 6 - move AccurateSeekTask::AdjustFastSeekIfNeeded();

Review commit: https://reviewboard.mozilla.org/r/98028/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98028/

Comment 7

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 7 - move AccurateSeekTask::Drop{Audio,Video}UpToSeekTarget();

Review commit: https://reviewboard.mozilla.org/r/98030/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98030/

Comment 8

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 8 - move AccurateSeekTask::MaybeFinishSeek();

Review commit: https://reviewboard.mozilla.org/r/98032/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98032/

Comment 9

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 9 - move AccurateSeekTask::CalculateNewCurrentTime();

Review commit: https://reviewboard.mozilla.org/r/98034/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98034/

Comment 10

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 10 - move all AccurateSeekTask data members;

Review commit: https://reviewboard.mozilla.org/r/98036/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98036/

Comment 11

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

Review commit: https://reviewboard.mozilla.org/r/98038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98038/

Comment 12

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1322799 part 12 - remove AccurateSeekTask;

Review commit: https://reviewboard.mozilla.org/r/98040/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98040/

Comment 13

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817765 [details]
Bug 1322799 part 3 - move AccurateSeekTask::OnSeek{Resoved,Rejected}();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98022/diff/1-2/

Comment 14

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817766 [details]
Bug 1322799 part 4 - move AccurateSeekTask::Request{Audio,Video}Data();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98024/diff/1-2/

Comment 15

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817767 [details]
Bug 1322799 part 5 - move AccurateSeekTask::Handle{Audio,Video,Not}{Decoded,Waited}(); }

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98026/diff/1-2/

Comment 16

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817768 [details]
Bug 1322799 part 6 - move AccurateSeekTask::AdjustFastSeekIfNeeded();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98028/diff/1-2/

Comment 17

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817769 [details]
Bug 1322799 part 7 - move AccurateSeekTask::Drop{Audio,Video}UpToSeekTarget();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98030/diff/1-2/

Comment 18

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817770 [details]
Bug 1322799 part 8 - move AccurateSeekTask::MaybeFinishSeek();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98032/diff/1-2/

Comment 19

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817771 [details]
Bug 1322799 part 9 - move AccurateSeekTask::CalculateNewCurrentTime();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98034/diff/1-2/

Comment 20

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817772 [details]
Bug 1322799 part 10 - move all AccurateSeekTask data members;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98036/diff/1-2/

Comment 21

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98038/diff/1-2/

Comment 22

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817774 [details]
Bug 1322799 part 12 - remove AccurateSeekTask;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98040/diff/1-2/

Comment 23

[Tzuhao Kuo [:kaku]]

Try looks bad...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=15a854612c2611f34499883276853d10753cdf6d
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a9b20c589d20ae3d6f261b20fe1db53fbd97d59

Comment 24

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817763 [details]
Bug 1322799 part 1 - un-encapsulating SeekTask;

https://reviewboard.mozilla.org/r/98018/#review98498

Comment 25

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817764 [details]
Bug 1322799 part 2 - let AccurateSeekingState request demuxer seeking;

https://reviewboard.mozilla.org/r/98020/#review98500

Comment 26

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817765 [details]
Bug 1322799 part 3 - move AccurateSeekTask::OnSeek{Resoved,Rejected}();

https://reviewboard.mozilla.org/r/98022/#review98502

Comment 27

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817766 [details]
Bug 1322799 part 4 - move AccurateSeekTask::Request{Audio,Video}Data();

https://reviewboard.mozilla.org/r/98024/#review98504

Comment 28

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817768 [details]
Bug 1322799 part 6 - move AccurateSeekTask::AdjustFastSeekIfNeeded();

https://reviewboard.mozilla.org/r/98028/#review98506

Comment 29

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817769 [details]
Bug 1322799 part 7 - move AccurateSeekTask::Drop{Audio,Video}UpToSeekTarget();

https://reviewboard.mozilla.org/r/98030/#review98508

Comment 30

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817770 [details]
Bug 1322799 part 8 - move AccurateSeekTask::MaybeFinishSeek();

https://reviewboard.mozilla.org/r/98032/#review98510

Comment 31

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817771 [details]
Bug 1322799 part 9 - move AccurateSeekTask::CalculateNewCurrentTime();

https://reviewboard.mozilla.org/r/98034/#review98512

Comment 32

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817767 [details]
Bug 1322799 part 5 - move AccurateSeekTask::Handle{Audio,Video,Not}{Decoded,Waited}(); }

https://reviewboard.mozilla.org/r/98026/#review98514

Comment 33

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817772 [details]
Bug 1322799 part 10 - move all AccurateSeekTask data members;

https://reviewboard.mozilla.org/r/98036/#review98516

Comment 34

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

https://reviewboard.mozilla.org/r/98038/#review98518

::: dom/media/MediaDecoderStateMachine.cpp
(Diff revision 2)
>      mSeekRequest.DisconnectIfExists();
>    }
>  
>    void HandleAudioDecoded(MediaData* aAudio) override
>    {
> -    MOZ_ASSERT(mSeekTaskRequest.Exists(), "Seek shouldn't be finished");

Why most of the assertions are removed?

Comment 35

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817774 [details]
Bug 1322799 part 12 - remove AccurateSeekTask;

https://reviewboard.mozilla.org/r/98040/#review98520

Comment 36

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

https://reviewboard.mozilla.org/r/98038/#review98518

> Why most of the assertions are removed?

Oh! I forgot to add it back.
The mSeekTaskRequest no longer holds any request. We should be able to modify the assertion to be: `MOZ_ASSERT(!mDoneAudioSeeking || !mDoneVideoSeeking)`. WDYT?

Comment 37

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

https://reviewboard.mozilla.org/r/98038/#review98518

> Oh! I forgot to add it back.
> The mSeekTaskRequest no longer holds any request. We should be able to modify the assertion to be: `MOZ_ASSERT(!mDoneAudioSeeking || !mDoneVideoSeeking)`. WDYT?

SGTM.

Comment 38

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817763 [details]
Bug 1322799 part 1 - un-encapsulating SeekTask;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98018/diff/1-2/

Comment 39

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817764 [details]
Bug 1322799 part 2 - let AccurateSeekingState request demuxer seeking;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98020/diff/1-2/

Comment 40

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817765 [details]
Bug 1322799 part 3 - move AccurateSeekTask::OnSeek{Resoved,Rejected}();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98022/diff/2-3/

Comment 41

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817766 [details]
Bug 1322799 part 4 - move AccurateSeekTask::Request{Audio,Video}Data();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98024/diff/2-3/

Comment 42

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817767 [details]
Bug 1322799 part 5 - move AccurateSeekTask::Handle{Audio,Video,Not}{Decoded,Waited}(); }

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98026/diff/2-3/

Comment 43

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817768 [details]
Bug 1322799 part 6 - move AccurateSeekTask::AdjustFastSeekIfNeeded();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98028/diff/2-3/

Comment 44

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817769 [details]
Bug 1322799 part 7 - move AccurateSeekTask::Drop{Audio,Video}UpToSeekTarget();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98030/diff/2-3/

Comment 45

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817770 [details]
Bug 1322799 part 8 - move AccurateSeekTask::MaybeFinishSeek();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98032/diff/2-3/

Comment 46

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817771 [details]
Bug 1322799 part 9 - move AccurateSeekTask::CalculateNewCurrentTime();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98034/diff/2-3/

Comment 47

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817772 [details]
Bug 1322799 part 10 - move all AccurateSeekTask data members;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98036/diff/2-3/

Comment 48

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98038/diff/2-3/

Comment 49

[Tzuhao Kuo [:kaku]]

Comment on attachment 8817774 [details]
Bug 1322799 part 12 - remove AccurateSeekTask;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98040/diff/2-3/

Comment 50

[Tzuhao Kuo [:kaku]]

Try again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b11abed3f4cf10ebe8ffc007164e56c011aafa22
https://treeherder.mozilla.org/#/jobs?repo=try&revision=402503d381a924ade9fc658a3e6945bc2b6af2ed

Comment 51

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

https://reviewboard.mozilla.org/r/98038/#review98810

Comment 52

[Tzuhao Kuo [:kaku]]

Thanks for the review! Try looks good.

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f5530626b38
part 1 - un-encapsulating SeekTask; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/435ed350aded
part 2 - let AccurateSeekingState request demuxer seeking; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/d3404f430dbe
part 3 - move AccurateSeekTask::OnSeek{Resoved,Rejected}(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/6aaada546c9e
part 4 - move AccurateSeekTask::Request{Audio,Video}Data(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/843ce5ebed5f
part 5 - move AccurateSeekTask::Handle{Audio,Video,Not}{Decoded,Waited}(); r=jwwang}
https://hg.mozilla.org/integration/autoland/rev/e9c176eb020f
part 6 - move AccurateSeekTask::AdjustFastSeekIfNeeded(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/aa32781d642e
part 7 - move AccurateSeekTask::Drop{Audio,Video}UpToSeekTarget(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/e1c21d0cf1e6
part 8 - move AccurateSeekTask::MaybeFinishSeek(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/7c0755de4751
part 9 - move AccurateSeekTask::CalculateNewCurrentTime(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/292f3801d9df
part 10 - move all AccurateSeekTask data members; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/3df49621bf13
part 11 - disconnect AccurateSeekingState and SeekTask; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/e40251a6b98f
part 12 - remove AccurateSeekTask; r=jwwang

Comment 54

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1f5530626b38
https://hg.mozilla.org/mozilla-central/rev/435ed350aded
https://hg.mozilla.org/mozilla-central/rev/d3404f430dbe
https://hg.mozilla.org/mozilla-central/rev/6aaada546c9e
https://hg.mozilla.org/mozilla-central/rev/843ce5ebed5f
https://hg.mozilla.org/mozilla-central/rev/e9c176eb020f
https://hg.mozilla.org/mozilla-central/rev/aa32781d642e
https://hg.mozilla.org/mozilla-central/rev/e1c21d0cf1e6
https://hg.mozilla.org/mozilla-central/rev/7c0755de4751
https://hg.mozilla.org/mozilla-central/rev/292f3801d9df
https://hg.mozilla.org/mozilla-central/rev/3df49621bf13
https://hg.mozilla.org/mozilla-central/rev/e40251a6b98f