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

()

Core
Audio/Video: Playback
RESOLVED DUPLICATE of bug 1340969
10 months ago
8 months ago

People

(Reporter: Treeherder Bug Filer, Assigned: jya)

Tracking

({intermittent-failure})

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
treeherder
Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=74982030&repo=mozilla-inbound

https://queue.taskcluster.net/v1/task/bkEvvh3KRpCsFxvAqjeVvQ/runs/0/artifacts/public/logs/live_backing.log
Fallout of bug 1319987?
Blocks: 1319987
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

10 months 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

10 months ago
well, you can take it then
Assignee: nobody → jwwang
(Assignee)

Comment 6

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

Comment 8

10 months 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

10 months 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

9 months ago
See Also: → bug 1340180

Comment 11

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