Closed Bug 1337265 Opened 5 years ago Closed 5 years ago

Intermittent dom/media/test/test_playback_errors.html | application crashed [@ mozilla::MediaFormatReader::DecoderFactory::RunStage] after Assertion failure: !data.mDecoder

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1340969

People

(Reporter: intermittent-bug-filer, Assigned: jya)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Fallout of bug 1319987?
Blocks: 1319987
Comment on attachment 8834436 [details]
Bug 1337265: Properly reset state when shutting down decoder.

https://reviewboard.mozilla.org/r/110386/#review111856

::: dom/media/MediaFormatReader.cpp:259
(Diff revision 2)
>  {
>    MOZ_ASSERT(aTrack == TrackInfo::kAudioTrack
>               || aTrack == TrackInfo::kVideoTrack);
> +
> +  auto& data = aTrack == TrackInfo::kAudioTrack ? mAudio : mVideo;
> +  if (data.mStage == Stage::CreateDecoder

https://hg.mozilla.org/integration/mozilla-inbound/file/f0453084d86e87070b2894eab61a2b58f1964768/dom/media/MediaFormatReader.cpp#l339

I don't think this is a right fix.

1. The check for |data.mStage == Stage::WaitForInit| doesn't help because we failed the assertion at #339 where data.mStage is Stage::CreateDecoder.
2. The check for |data.mStage == Stage::CreateDecoder| doesn't help either because the current call flow [a] ensures data.mStage is never Stage::CreateDecoder inside MediaFormatReader::DecoderFactory::CreateDecoder().

[a] data.mStage is changed to Stage::CreateDecoder at #320 and the following RunStage() changes it to Stage::WaitForInit immediately.
Attachment #8834436 - Flags: review?(jwwang) → review-
well, you can take it then
Assignee: nobody → jwwang
oh doh !
Assignee: jwwang → jyavenard
Comment on attachment 8834436 [details]
Bug 1337265: Properly reset state when shutting down decoder.

https://reviewboard.mozilla.org/r/110386/#review112218

::: dom/media/MediaFormatReader.cpp:219
(Diff revision 3)
>        // return the existing promise.
>        data.mShutdownRequest.Disconnect();
>        RefPtr<ShutdownPromise> p = data.mShutdownPromise.forget();
>        return p;
>      }
> +    data.mStage = Stage::None;

I can't see how this will help.

Assertion failure: !data.mDecoder, at /home/worker/workspace/build/src/dom/media/MediaFormatReader.cpp:339

#220 clear data.mDecoder so it is impossible to fail the assertion if this function is ever called.
http://searchfox.org/mozilla-central/rev/672c83ed65da286b68be1d02799c35fdd14d0134/dom/media/MediaFormatReader.cpp#202

Can we just ignore the return value of MediaDataDecoder::Shutdown() in this case? It appears to me that MFR only cares about when to call Shutdown() on a MediaDataDecoder instead of when Shutdown() is done.

http://searchfox.org/mozilla-central/rev/672c83ed65da286b68be1d02799c35fdd14d0134/dom/media/MediaFormatReader.cpp#2625

Likewise, ReleaseResources() also ignores the shutdown promise returned by MediaDataDecoder::Shutdown().

We will be able to make the shutdown sequence much easier if we can ignore the return value of MediaDataDecoder::Shutdown() in most cases.
Comment on attachment 8834436 [details]
Bug 1337265: Properly reset state when shutting down decoder.

https://reviewboard.mozilla.org/r/110386/#review112218

> I can't see how this will help.
> 
> Assertion failure: !data.mDecoder, at /home/worker/workspace/build/src/dom/media/MediaFormatReader.cpp:339
> 
> #220 clear data.mDecoder so it is impossible to fail the assertion if this function is ever called.

Exactly. It asserts that mDecoder exists when it doesn't as it's been shutdown before init completed. Yet the state would still be WaitingForInit

The MFR calls shutdown while an init is in progress and then attempt to recreate one. This go straight to WaitingForInit which will assert.
Component: Audio/Video → Audio/Video: Playback
See Also: → 1340180
Comment on attachment 8834436 [details]
Bug 1337265: Properly reset state when shutting down decoder.

https://reviewboard.mozilla.org/r/110386/#review114072

It looks like the issue has been fixed by bug 1340969.
Attachment #8834436 - Flags: review?(jwwang)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1340969
You need to log in before you can comment on or make changes to this bug.