Closed Bug 1322799 Opened 5 years ago Closed 5 years ago

Remove AccurateSeekTask

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: kaku, Assigned: kaku)

References

Details

Attachments

(12 files)

58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
No description provided.
Assignee: nobody → kaku
Blocks: 1318610
Depends on: 1322801
Comment on attachment 8817763 [details]
Bug 1322799 part 1 - un-encapsulating SeekTask;

https://reviewboard.mozilla.org/r/98018/#review98498
Attachment #8817763 - Flags: review?(jwwang) → review+
Comment on attachment 8817764 [details]
Bug 1322799 part 2 - let AccurateSeekingState request demuxer seeking;

https://reviewboard.mozilla.org/r/98020/#review98500
Attachment #8817764 - Flags: review?(jwwang) → review+
Comment on attachment 8817765 [details]
Bug 1322799 part 3 - move AccurateSeekTask::OnSeek{Resoved,Rejected}();

https://reviewboard.mozilla.org/r/98022/#review98502
Attachment #8817765 - Flags: review?(jwwang) → review+
Comment on attachment 8817766 [details]
Bug 1322799 part 4 - move AccurateSeekTask::Request{Audio,Video}Data();

https://reviewboard.mozilla.org/r/98024/#review98504
Attachment #8817766 - Flags: review?(jwwang) → review+
Comment on attachment 8817768 [details]
Bug 1322799 part 6 - move AccurateSeekTask::AdjustFastSeekIfNeeded();

https://reviewboard.mozilla.org/r/98028/#review98506
Attachment #8817768 - Flags: review?(jwwang) → review+
Comment on attachment 8817769 [details]
Bug 1322799 part 7 - move AccurateSeekTask::Drop{Audio,Video}UpToSeekTarget();

https://reviewboard.mozilla.org/r/98030/#review98508
Attachment #8817769 - Flags: review?(jwwang) → review+
Comment on attachment 8817770 [details]
Bug 1322799 part 8 - move AccurateSeekTask::MaybeFinishSeek();

https://reviewboard.mozilla.org/r/98032/#review98510
Attachment #8817770 - Flags: review?(jwwang) → review+
Comment on attachment 8817771 [details]
Bug 1322799 part 9 - move AccurateSeekTask::CalculateNewCurrentTime();

https://reviewboard.mozilla.org/r/98034/#review98512
Attachment #8817771 - Flags: review?(jwwang) → review+
Comment on attachment 8817767 [details]
Bug 1322799 part 5 - move AccurateSeekTask::Handle{Audio,Video,Not}{Decoded,Waited}(); }

https://reviewboard.mozilla.org/r/98026/#review98514
Attachment #8817767 - Flags: review?(jwwang) → review+
Comment on attachment 8817772 [details]
Bug 1322799 part 10 - move all AccurateSeekTask data members;

https://reviewboard.mozilla.org/r/98036/#review98516
Attachment #8817772 - Flags: review?(jwwang) → review+
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 on attachment 8817774 [details]
Bug 1322799 part 12 - remove AccurateSeekTask;

https://reviewboard.mozilla.org/r/98040/#review98520
Attachment #8817774 - Flags: review?(jwwang) → review+
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 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 on attachment 8817773 [details]
Bug 1322799 part 11 - disconnect AccurateSeekingState and SeekTask;

https://reviewboard.mozilla.org/r/98038/#review98810
Attachment #8817773 - Flags: review?(jwwang) → review+
Thanks for the review! Try looks good.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
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
Keywords: checkin-needed
Blocks: 1323931
Duplicate of this bug: 1322999
You need to log in before you can comment on or make changes to this bug.