Don't switch callbacks between MDSM and SeekTask

RESOLVED FIXED in Firefox 53

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kaku, Assigned: kaku)

Tracking

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(8 attachments)

(Assignee)

Description

2 years ago
A sub-bug of bug 1318610. 

We implemented the mechanism of switching MediaDecoderReaderWarpper's callbacks between MDSM and SeekTasks in bug 1261020. Now, in bug 1318610, we're going to get rid of the SeekTasks and this bug is the first step to aligning the behavior of SeekTask to SeekingState.
(Assignee)

Updated

2 years ago
Assignee: nobody → kaku
Blocks: 1318610
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8814594 [details]
Bug 1320466 part 1 - delegate OnNotDecoded event to state objects;

https://reviewboard.mozilla.org/r/95776/#review95838

::: dom/media/MediaDecoderStateMachine.cpp:1165
(Diff revision 1)
> +MediaDecoderStateMachine::
> +StateObject::HandleNotDecoded(MediaData::Type aType, const MediaResult& aError)
> +{
> +  bool isAudio = aType == MediaData::AUDIO_DATA;
> +  MOZ_ASSERT_IF(!isAudio, aType == MediaData::VIDEO_DATA);
> +

The check for IsShutdown() is missing here. Isn't that needed anymore?

Comment 11

2 years ago
mozreview-review
Comment on attachment 8814595 [details]
Bug 1320466 part 2 - implement media data decoded/not-decoded handlers in SeekTask;

https://reviewboard.mozilla.org/r/95778/#review95840
Attachment #8814595 - Flags: review?(jwwang) → review+

Comment 12

2 years ago
mozreview-review
Comment on attachment 8814596 [details]
Bug 1320466 part 3 - implement OnAudioWaited(), OnVideoWaited() and OnNotWaited() callbacks in MDSM;

https://reviewboard.mozilla.org/r/95780/#review95842
Attachment #8814596 - Flags: review?(jwwang) → review+

Comment 13

2 years ago
mozreview-review
Comment on attachment 8814597 [details]
Bug 1320466 part 4 - delegate OnAudioWaited, OnVideoWaited and OnNotWaited events to state objects;

https://reviewboard.mozilla.org/r/95782/#review95844
Attachment #8814597 - Flags: review?(jwwang) → review+

Comment 14

2 years ago
mozreview-review
Comment on attachment 8814598 [details]
Bug 1320466 part 5 - implement media data waited/not-waited handlers in SeekTask;

https://reviewboard.mozilla.org/r/95784/#review95846
Attachment #8814598 - Flags: review?(jwwang) → review+

Comment 15

2 years ago
mozreview-review
Comment on attachment 8814599 [details]
Bug 1320466 part 6 - dispatch {not-}decoded and {not-}waited events from SeekingObject to SeekTask;

https://reviewboard.mozilla.org/r/95786/#review95848
Attachment #8814599 - Flags: review?(jwwang) → review+

Comment 16

2 years ago
mozreview-review
Comment on attachment 8814600 [details]
Bug 1320466 part 7 - remove callbacks of SeekTask;

https://reviewboard.mozilla.org/r/95788/#review95850

Nice!
Attachment #8814600 - Flags: review?(jwwang) → review+

Comment 17

2 years ago
mozreview-review
Comment on attachment 8814601 [details]
Bug 1320466 part 8 - clean up redundant fucntion calls;

https://reviewboard.mozilla.org/r/95790/#review95852
Attachment #8814601 - Flags: review?(jwwang) → review+
(Assignee)

Comment 18

2 years ago
mozreview-review-reply
Comment on attachment 8814594 [details]
Bug 1320466 part 1 - delegate OnNotDecoded event to state objects;

https://reviewboard.mozilla.org/r/95776/#review95838

> The check for IsShutdown() is missing here. Isn't that needed anymore?

I handle it by overriding the HandleNotDecoded() method in ShutdownState.

ShutdownState::HandleNotDecoded() returns directily.
(Assignee)

Updated

2 years ago
Blocks: 1321140
(Assignee)

Updated

2 years ago
Blocks: 1321142

Comment 19

2 years ago
mozreview-review
Comment on attachment 8814594 [details]
Bug 1320466 part 1 - delegate OnNotDecoded event to state objects;

https://reviewboard.mozilla.org/r/95776/#review96998
Attachment #8814594 - Flags: review?(jwwang) → review+
(Assignee)

Comment 20

2 years ago
Thanks for the review!
Keywords: checkin-needed

Comment 21

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d8a14192979c
part 1 - delegate OnNotDecoded event to state objects; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/3af879aacac7
part 2 - implement media data decoded/not-decoded handlers in SeekTask; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/b40ed2eb5cb5
part 3 - implement OnAudioWaited(), OnVideoWaited() and OnNotWaited() callbacks in MDSM; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/e102e1d94504
part 4 - delegate OnAudioWaited, OnVideoWaited and OnNotWaited events to state objects; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/38c2d58de393
part 5 - implement media data waited/not-waited handlers in SeekTask; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/2db94d238128
part 6 - dispatch {not-}decoded and {not-}waited events from SeekingObject to SeekTask; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/3a13ac4fb7a6
part 7 - remove callbacks of SeekTask; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/98428e833421
part 8 - clean up redundant fucntion calls; r=jwwang
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.