1320466 - Don't switch callbacks between MDSM and SeekTask

Status: RESOLVED FIXED

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

Description

[Tzuhao Kuo [:kaku]]

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.

Comment 1

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 1 - delegate OnNotDecoded event to state objects;

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

Comment 2

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 2 - implement media data decoded/not-decoded handlers in SeekTask;

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

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 3 - implement OnAudioWaited(), OnVideoWaited() and OnNotWaited() callbacks in MDSM;

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

Comment 4

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 4 - delegate OnAudioWaited, OnVideoWaited and OnNotWaited events to state objects;

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

Comment 5

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 5 - implement media data waited/not-waited handlers in SeekTask;

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

Comment 6

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 6 - dispatch {not-}decoded and {not-}waited events from SeekingObject to SeekTask;

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

Comment 7

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 7 - remove callbacks of SeekTask;

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

Comment 8

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1320466 part 8 - clean up redundant fucntion calls;

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

Comment 9

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bf6fb9e34dcea7b7f627b4dc1d6174ee58da3bf
https://treeherder.mozilla.org/#/jobs?repo=try&revision=33e3e7c708e3243629b4da548d5ea6c03dee1f36

Comment 10

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

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

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

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

Comment 12

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

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

https://reviewboard.mozilla.org/r/95780/#review95842

Comment 13

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

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

Comment 14

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

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

Comment 15

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

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

Comment 16

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

Comment on attachment 8814600 [details]
Bug 1320466 part 7 - remove callbacks of SeekTask;

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

Nice!

Comment 17

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

Comment on attachment 8814601 [details]
Bug 1320466 part 8 - clean up redundant fucntion calls;

https://reviewboard.mozilla.org/r/95790/#review95852

Comment 18

[Tzuhao Kuo [:kaku]]

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.

Comment 19

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

Comment on attachment 8814594 [details]
Bug 1320466 part 1 - delegate OnNotDecoded event to state objects;

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

Comment 20

[Tzuhao Kuo [:kaku]]

Thanks for the review!

Comment 21

[Pulsebot]

Pushed by [e-mail]:
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

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d8a14192979c
https://hg.mozilla.org/mozilla-central/rev/3af879aacac7
https://hg.mozilla.org/mozilla-central/rev/b40ed2eb5cb5
https://hg.mozilla.org/mozilla-central/rev/e102e1d94504
https://hg.mozilla.org/mozilla-central/rev/38c2d58de393
https://hg.mozilla.org/mozilla-central/rev/2db94d238128
https://hg.mozilla.org/mozilla-central/rev/3a13ac4fb7a6
https://hg.mozilla.org/mozilla-central/rev/98428e833421