1238906 - Implement audible data checking in the MDSM's audio queue

Status: RESOLVED FIXED

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

Description

[Alastor Wu [:alwu]]

Implement the way that MediaElement can be notified if its audio track doesn't produce any sound.

Comment 1

[Alastor Wu [:alwu]]

Attachment: MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

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

Comment 2

[Alastor Wu [:alwu]]

Attachment: MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

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

Comment 3

[Alastor Wu [:alwu]]

Hi, JW,
Could you help me review this patch?

It implements the way that MediaElement can be notified if its audio track doesn't produce any sound. MDSM would check whether the audio queue has too much silent data that we can regard it as a silent queue.

Very appreciate!

Comment 4

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

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

https://reviewboard.mozilla.org/r/30513/#review27471

::: dom/media/MediaData.cpp:58
(Diff revision 1)
> +AudioData::IsAudible() const

Can you run a profiling to ensure it doesn't incur performance hit?

::: dom/media/MediaDecoderStateMachine.cpp:128
(Diff revision 1)
> +static const int SILENT_DATA_THRESHOLD_USECS = 30000;

You are comparing a signed integer with an unsigned one (mSilentDataDuration).

::: dom/media/MediaDecoderStateMachine.cpp:737
(Diff revision 1)
> +MediaDecoderStateMachine::OnAudioPushed(const RefPtr<MediaData>& aSample)

You should do the check in OnAudioPopped() which is called when audio data is consumed.

Comment 5

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

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

https://reviewboard.mozilla.org/r/30515/#review27473

::: dom/html/HTMLMediaElement.h:1528
(Diff revision 1)
> +  // Sometime the audio track may have only silent data.

The comment doesn't explain the purpose and usage of this member.

::: dom/html/HTMLMediaElement.cpp:5176
(Diff revision 1)
> +  if (mIsAudioTrackAudible != aAudible) {

You can just assign |aAudible| to |mIsAudioTrackAudible| without the if statement.

::: dom/media/MediaDecoder.h:230
(Diff revision 1)
> +// declartion. If we use the enum class, it can't be used in the operator==.

Can you elaborate why operator== can't be used?

::: dom/media/MediaDecoder.h:232
(Diff revision 1)
> +  NONE_INIT,

Why do we need NONE_INIT? Won't AUDIBLE and SILENT do the work?

::: dom/media/MediaDecoderStateMachine.cpp:290
(Diff revision 1)
> +  mIsAudioDataAudible(mTaskQueue, AudibleState::NONE_INIT,

Can it be initialized to AudibleState::SILENT?

Comment 6

[Alastor Wu [:alwu]]

(In reply to JW Wang [:jwwang] from comment #4)
> Comment on attachment 8706857 [details]
> MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.
> 
> https://reviewboard.mozilla.org/r/30513/#review27471
> 
> ::: dom/media/MediaData.cpp:58
> (Diff revision 1)
> > +AudioData::IsAudible() const
> 
> Can you run a profiling to ensure it doesn't incur performance hit?

Sure, I'll update info after finishing profiling.

Comment 7

[Alastor Wu [:alwu]]

(In reply to JW Wang [:jwwang] from comment #5)
> Comment on attachment 8706858 [details]
> MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to
> ME.
> 
> https://reviewboard.mozilla.org/r/30515/#review27473
> 
> ::: dom/html/HTMLMediaElement.h:1528
> (Diff revision 1)
> > +  // Sometime the audio track may have only silent data.
> 
> The comment doesn't explain the purpose and usage of this member.

I'll remove this enum, because I found that I don't need AudibleState::NONE_INIT.

> ::: dom/html/HTMLMediaElement.cpp:5176
> (Diff revision 1)
> > +  if (mIsAudioTrackAudible != aAudible) {
> 
> You can just assign |aAudible| to |mIsAudioTrackAudible| without the if
> statement.

That is because I'll do something in this brackets in the follow-up patches (bug1235612).
 
> ::: dom/media/MediaDecoder.h:230
> (Diff revision 1)
> > +// declartion. If we use the enum class, it can't be used in the operator==.
> 
> Can you elaborate why operator== can't be used?

Remove AudibleState.

> ::: dom/media/MediaDecoder.h:232
> (Diff revision 1)
> > +  NONE_INIT,
> 
> Why do we need NONE_INIT? Won't AUDIBLE and SILENT do the work?

Ditto.

> ::: dom/media/MediaDecoderStateMachine.cpp:290
> (Diff revision 1)
> > +  mIsAudioDataAudible(mTaskQueue, AudibleState::NONE_INIT,
> 
> Can it be initialized to AudibleState::SILENT?

Ditto.

Comment 8

[Alastor Wu [:alwu]]

Here is the profiling result, I do the test using Perf on Linux.
Since we did some extra checking in the OnAudioPopped(), we can check its performance.
Strangely, it's even better after applying these patches from the perf result.

[1] Before : https://goo.gl/ftqNWx
[2] After : https://goo.gl/hHWltC

Comment 9

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

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

Comment 10

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

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

Comment 11

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

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

https://reviewboard.mozilla.org/r/30513/#review27625

::: dom/media/MediaDecoderStateMachine.cpp:740
(Diff revisions 1 - 2)
> -  RefPtr<AudioData> data = aSample->As<AudioData>();
> +  if (aData->IsAudible() && !mIsAudioDataAudible) {

You should cache the result of IsAudible() which might be heavy.

::: dom/media/MediaDecoderStateMachine.cpp:762
(Diff revisions 1 - 2)
> +  UpdateAudibleStatus(aSample->As<AudioData>());

Move the cast into UpdateAudibleStatus() and remove the assertion above so OnAudioPopped() is less coupled with aSample->mType.

Comment 12

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

(In reply to Alastor Wu [:alwu] from comment #8)
> Here is the profiling result, I do the test using Perf on Linux.
> Since we did some extra checking in the OnAudioPopped(), we can check its
> performance.
> Strangely, it's even better after applying these patches from the perf
> result.
> 
> [1] Before : https://goo.gl/ftqNWx
> [2] After : https://goo.gl/hHWltC

The result looks strange to me. Can you do the profiling a few times and check if the results are consistent?

Comment 13

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

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

https://reviewboard.mozilla.org/r/30515/#review27629

::: dom/media/MediaDecoderStateMachine.h:415
(Diff revisions 1 - 2)
> +  void CheckIsAudible(const RefPtr<AudioData> aData);

aData can take a raw pointer to avoid ref-counting overhead.

Comment 14

[Alastor Wu [:alwu]]

It’s not significant performance difference after applying this patch. [1]

The testing procedure is to play a normal mp3 file [2] which duration is 3:49. The platform is linux, and the profiling tool is PERF. 

Testing STR.
1. $ perf record ./mach run [test file]
2. playing audio until it ended
3. $ perf report
4. filter “MediaDecoderStateMachine”, and find the exec-time of the OnAudioPopped(). 

[Result unit is the exec-time-proportion of the whole MediaDecoderStateMachine]
* Before applying this patch : avg=4.16 sd=2.01
* After applying this patch  : avg=4.30 sd=1.91

[1] Result screen shot : https://goo.gl/lFcmWe
[2] Test audio file : https://goo.gl/bQhL5Y

Comment 15

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

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

Comment 16

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

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

Comment 17

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

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

Comment 18

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

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

Comment 19

[Alastor Wu [:alwu]]

Cancel review because of some test case fails, I'll ask review after fixing them.

Comment 20

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30513/diff/4-5/

Comment 21

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30515/diff/4-5/

Comment 22

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

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

https://reviewboard.mozilla.org/r/30513/#review27843

::: dom/media/MediaData.h:136
(Diff revisions 2 - 5)
> +    if (mAudioData) {

It is a waste to compute IsAudible() in the constructor since it might not even be consumed by MDSM.

::: dom/media/MediaData.cpp:62
(Diff revisions 2 - 5)
> +#ifdef MOZ_SAMPLE_TYPE_S16

Why do you have 2 code paths? Won't it work to compare the value with 0?

Comment 23

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30513/diff/5-6/

Comment 24

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30515/diff/5-6/

Comment 25

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

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

https://reviewboard.mozilla.org/r/30513/#review27851

::: dom/media/MediaData.h:133
(Diff revisions 2 - 6)
> -    , mAudioData(Move(aData)) {}
> +    , mAudioData(Move(aData))

unnecessary change.

::: dom/media/MediaData.h:166
(Diff revisions 2 - 6)
> -  ~AudioData() {}
> +  ~AudioData() {};

unnecessary change.

::: dom/media/MediaData.cpp:63
(Diff revisions 2 - 6)
> -      if (mAudioData[frame * mChannels + channel] != 0) {
> +      if (mAudioData[frame * mChannels + channel] != 0.0) {

say 0 instead of 0.0 to avoid floating point conversion overhead when MOZ_SAMPLE_TYPE_S16 is defined.

::: dom/media/MediaDecoderStateMachine.cpp:748
(Diff revisions 2 - 6)
> -  UpdateAudibleStatus(aSample->As<AudioData>());
> +  CheckIsAudible(aSample.get());

Just say |aSample| which is implicitly convertible to  MediaData*.

Comment 26

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30513/diff/6-7/

Comment 27

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30515/diff/6-7/

Comment 28

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

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

https://reviewboard.mozilla.org/r/30513/#review27973

Comment 29

[Alastor Wu [:alwu]]

Thanks for review!

---

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d620b6bb5c2

Comment 30

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1857bca40ac4
https://hg.mozilla.org/integration/mozilla-inbound/rev/e729b30ba7b4

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for M(2) bustage on debug builds of OSX and Windows: 

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/cdc15e930dab

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e729b30ba7b4

Test run with failure: https://treeherder.mozilla.org/logviewer.html#?job_id=19979097&repo=mozilla-inbound
02:28:31     INFO -  Assertion failure: mAudioData, at c:/builds/moz2_slave/m-in-w64-d-0000000000000000000/build/src/dom/media/MediaData.cpp:60
02:28:46     INFO -  #01: mozilla::detail::ListenerHelper<mozilla::AbstractThread,<lambda_6e4d443be27a1ff4b0ac1d3d27cbff91> >::R<RefPtr<mozilla::MediaData> const &>::Run() [dom/media/MediaEventSource.h:155]
02:28:46     INFO -  #02: mozilla::TaskQueue::Runner::Run() [xpcom/threads/TaskQueue.cpp:171]
02:28:46     INFO -  #03: nsThreadPool::Run() [xpcom/threads/nsThreadPool.cpp:223]
02:28:46     INFO -  #04: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:989]
02:28:46     INFO -  #05: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/glue/nsThreadUtils.cpp:297]
02:28:46     INFO -  #06: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:326]
02:28:46     INFO -  #07: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:228]
02:28:46     INFO -  #08: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:202]
02:28:46     INFO -  #09: nsThread::ThreadFunc(void *) [xpcom/threads/nsThread.cpp:403]
02:28:46     INFO -  #10: PR_NativeRunThread [nsprpub/pr/src/threads/combined/pruthr.c:406]
02:28:46     INFO -  #11: pr_root [nsprpub/pr/src/md/windows/w95thred.c:91]
02:28:46     INFO -  #12: MSVCR120 + 0x24f7f
02:28:46     INFO -  #13: MSVCR120 + 0x25126
02:28:46     INFO -  #14: KERNEL32 + 0x167e
02:28:46     INFO -  #15: ntdll + 0x1c3f1
02:28:46     INFO -  TEST-INFO | Main app process: exit 1
02:28:46     INFO -  44861 INFO TEST-PASS | dom/media/test/test_buffered.html | A valid string reason is expected
...
02:28:46     INFO -  44915 INFO TEST-PASS | dom/media/test/test_buffered.html | [started bug516323.indexed.ogv-6 t=1.267] Length of array should match number of running tests
02:28:46  WARNING -  TEST-UNEXPECTED-FAIL | dom/media/test/test_buffered.html | application terminated with exit code 1
02:28:46     INFO -  runtests.py | Application ran for: 0:00:58.585000
02:28:46     INFO -  zombiecheck | Reading PID log: c:\users\cltbld~1.t-w\appdata\local\temp\tmpiqeiitpidlog
02:28:46     INFO -  mozcrash Copy/paste: C:\slave\test\build\win32-minidump_stackwalk.exe c:\users\cltbld~1.t-w\appdata\local\temp\tmpiolgcq.mozrunner\minidumps\1e4b8ba8-6c61-4567-8005-b90537cd54f7.dmp C:\slave\test\build\symbols
02:28:55     INFO -  mozcrash Saved minidump as C:\slave\test\build\blobber_upload_dir\1e4b8ba8-6c61-4567-8005-b90537cd54f7.dmp
02:28:55     INFO -  mozcrash Saved app info as C:\slave\test\build\blobber_upload_dir\1e4b8ba8-6c61-4567-8005-b90537cd54f7.extra
02:28:55  WARNING -  PROCESS-CRASH | dom/media/test/test_buffered.html | application crashed [@ mozilla::AudioData::IsAudible()]
02:28:55     INFO -  Crash dump filename: c:\users\cltbld~1.t-w\appdata\local\temp\tmpiolgcq.mozrunner\minidumps\1e4b8ba8-6c61-4567-8005-b90537cd54f7.dmp
02:28:55     INFO -  Operating system: Windows NT
02:28:55     INFO -                    6.2.9200
02:28:55     INFO -  CPU: amd64
02:28:55     INFO -       family 6 model 30 stepping 5
02:28:55     INFO -       8 CPUs
02:28:55     INFO -  Crash reason:  EXCEPTION_BREAKPOINT
02:28:55     INFO -  Crash address: 0x7feba1dafdc
02:28:55     INFO -  Process uptime: 44 seconds
02:28:55     INFO -  Thread 75 (crashed)
02:28:55     INFO -   0  xul.dll!mozilla::AudioData::IsAudible() [MediaData.cpp:e729b30ba7b4 : 60 + 0x62]
02:28:55     INFO -      rax = 0x0000000000000000   rdx = 0x0000000000000002
02:28:55     INFO -      rcx = 0x000007febd123548   rbx = 0x000000000000003c
02:28:55     INFO -      rsi = 0x000000e5420961a0   rdi = 0x000000e5420961a0
02:28:55     INFO -      rbp = 0x000000e54250a460   rsp = 0x000000e55013f7a0
02:28:55     INFO -       r8 = 0x000007feca9119b0    r9 = 0x000007feca9119b0
02:28:55     INFO -      r10 = 0x0000000000000000   r11 = 0x000000e5501391e0
02:28:55     INFO -      r12 = 0x000000e5429b2510   r13 = 0x000000e55013fb00
02:28:55     INFO -      r14 = 0x000000e5426ba700   r15 = 0x0000000000000000
02:28:55     INFO -      rip = 0x000007feba1dafdc
02:28:55     INFO -      Found by: given as instruction pointer in context

Comment 32

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30513/diff/7-8/

Comment 33

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30515/diff/7-8/

Comment 34

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3face68bc4a6

Comment 35

[Carsten Book [:Tomcat]]

part 2 failed to apply:

30567afa89aa transplanted to 40a421ac4a7f
applying 849eb1555126
patching file dom/media/gtest/MockMediaDecoderOwner.h
Hunk #1 FAILED at 41
1 out of 1 hunks FAILED -- saving rejects to file dom/media/gtest/MockMediaDecoderOwner.h.rej
patch failed to apply
abort: fix up the merge and run hg transplant --continue

Comment 36

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30513/diff/8-9/

Comment 37

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30515/diff/8-9/

Comment 38

[Alastor Wu [:alwu]]

Here is the rebased patch, thanks!

Comment 39

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/d5b8283acbce
https://hg.mozilla.org/integration/b2g-inbound/rev/03528baabf55

Comment 40

[Carsten Book [:Tomcat]]

(In reply to Pulsebot from comment #39)
> https://hg.mozilla.org/integration/b2g-inbound/rev/d5b8283acbce
> https://hg.mozilla.org/integration/b2g-inbound/rev/03528baabf55

sorry had to back this out for blocking/conflict with the merge down from mozilla-central in warning: conflicts while merging dom/media/gtest/MockMediaDecoderOwner.h! (edit, then use 'hg resolve --mark')

Comment 41

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/b2g-inbound/rev/2de2be804461
https://hg.mozilla.org/integration/b2g-inbound/rev/cd5c912951c2

Comment 42

[Alastor Wu [:alwu]]

Comment on attachment 8706857 [details]
MozReview Request: Bug 1238906 - part1 : check whether audio data is audible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30513/diff/9-10/

Comment 43

[Alastor Wu [:alwu]]

Comment on attachment 8706858 [details]
MozReview Request: Bug 1238906 - part2 : notify audible state from MDSM to ME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30515/diff/9-10/

Comment 44

[Alastor Wu [:alwu]]

Rebased, thanks!

Comment 45

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c5cdbfeb6a3
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d17eb993510

Comment 46

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

https://hg.mozilla.org/mozilla-central/rev/5c5cdbfeb6a3
https://hg.mozilla.org/mozilla-central/rev/4d17eb993510

Comment 47

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

https://reviewboard.mozilla.org/r/30515/#review60950

::: dom/media/MediaDecoder.cpp:630
(Diff revision 10)
>      mFirstFrameLoadedListener.Disconnect();
>      mOnPlaybackEvent.Disconnect();
>      mOnSeekingStart.Disconnect();
>      mOnMediaNotSeekable.Disconnect();
>  
> +    mWatchManager.Unwatch(mIsAudioDataAudible, &MediaDecoder::NotifyAudibleStateChanged);

Why do we need to call Unwatch() here?

Comment 48

[Alastor Wu [:alwu]]

We would crash at MediaDecoder if we remove the unwatch(). [1]

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=23814665&repo=try#L3057

Comment 49

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

We should disconnect the mirrors as early as MediaDecoder::Shutdown() begins instead of doing it in SetStateMachine(nullptr) which is called in MediaDecoder::FinishShutdown(). I will file a new bug to do that. Thanks for the clarification.