1292449 - Remove some more IsShutdown() checks from MediaDecoder

Status: RESOLVED FIXED

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

Description

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

The sequel of bug 1290780.

Comment 1

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

Attachment: Bug 1292449. Part 1 - Assert IsShutdown() is false in NotifyDataArrived().

1. Called from SourceBuffer::AppendDataCompletedWithSuccess() where mMediaSource->GetDecoder() is not null so this must happen before Shutdown().
2. Called from SourceBuffer::Ended() where mMediaSource->GetDecoder() is not null.
3. Called from MediaDecoder::ResourceCallback::TimerCallback() which is canceled in Disconnect().

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

Comment 2

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

Attachment: Bug 1292449. Part 2 - Assert IsShutdown() is false in NotifyBytesDownloaded().

1. Called from SourceBuffer::AppendDataCompletedWithSuccess() where mMediaSource->GetDecoder() is not null.
2. Called from MediaDecoder::ResourceCallback::NotifyBytesDownloaded() which is disconnected in Shutdown().

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

Comment 3

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

Attachment: Bug 1292449. Part 3 - Assert IsShutdown() is false in MediaDecoder::Shutdown(). Shutdown() should be only called once.

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

Comment 4

[Tzuhao Kuo [:kaku]]

Comment on attachment 8778119 [details]
Bug 1292449. Part 3 - Assert IsShutdown() is false in MediaDecoder::Shutdown(). Shutdown() should be only called once.

https://reviewboard.mozilla.org/r/69506/#review66998

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8778117 [details]
Bug 1292449. Part 1 - Assert IsShutdown() is false in NotifyDataArrived().

https://reviewboard.mozilla.org/r/69502/#review67004

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8778118 [details]
Bug 1292449. Part 2 - Assert IsShutdown() is false in NotifyBytesDownloaded().

https://reviewboard.mozilla.org/r/69504/#review67006

Comment 7

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

Thanks for the review!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c39c23a88c7a
Part 1 - Assert IsShutdown() is false in NotifyDataArrived(). r=jya
https://hg.mozilla.org/integration/autoland/rev/a62419846b2f
Part 2 - Assert IsShutdown() is false in NotifyBytesDownloaded(). r=jya
https://hg.mozilla.org/integration/autoland/rev/e47457d354ec
Part 3 - Assert IsShutdown() is false in MediaDecoder::Shutdown(). Shutdown() should be only called once. r=kaku

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/c39c23a88c7a
https://hg.mozilla.org/mozilla-central/rev/a62419846b2f
https://hg.mozilla.org/mozilla-central/rev/e47457d354ec