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

RESOLVED DUPLICATE of bug 1340969

Status

()

RESOLVED DUPLICATE of bug 1340969
2 years ago
2 years ago

People

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

Tracking

({intermittent-failure})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Fallout of bug 1319987?
Blocks: 1319987
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
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-
(Assignee)

Comment 5

2 years ago
well, you can take it then
Assignee: nobody → jwwang
(Assignee)

Comment 6

2 years ago
oh doh !
Assignee: jwwang → jyavenard
Comment hidden (mozreview-request)

Comment 8

2 years ago
mozreview-review
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.
(Assignee)

Comment 10

2 years ago
mozreview-review-reply
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
(Assignee)

Updated

2 years ago
See Also: → bug 1340180

Comment 11

2 years ago
mozreview-review
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
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1340969
You need to log in before you can comment on or make changes to this bug.