MP4Reader can stick output into MediaQueues after flush but before next Decode() call

RESOLVED FIXED in mozilla34

Status

()

Core
Audio/Video
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

(Blocks: 1 bug)

29 Branch
mozilla34
x86_64
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
Two small problems in MP4Reader:

1. If output is sitting in a runnable on a task queue waiting to be delivered to the MP4Reader's Output function, it can end be being stuck into the MediaQueue in MP4Reader::Output. This can disrupt seeking or shutdown.

2. In MP4Reader::ReadMetadata() the path that creates the CDMWrapper PDM relies on HasAudio() working, but HasAudio() won't work until after the "if (mDemuxer->HasValidAudio()) {" block further down, as that initializes mInfo.mAudio.mHasAudio and mAudio.mActive. We should initialize these earlier, otherwise the HasAudio() return value we pass to PlatformDecoderModule::CreateCDMWrapper() will be wrong, and we won't create a PDM when we need one.
(Assignee)

Comment 1

4 years ago
(In reply to Chris Pearce (:cpearce) from comment #0)
> Two small problems in MP4Reader:
> 
> 1. If output is sitting in a runnable on a task queue waiting to be
> delivered to the MP4Reader's Output function, it can end be being stuck into
> the MediaQueue in MP4Reader::Output. This can disrupt seeking or shutdown.

This doesn't disrupt seeking, at least not for the WMF PDM, there seems to be a bug in the demuxer that was causing my problems. We can still call MP4Reader::Output while flushing, and we need to delete the MediaData object passed to us if so, otherwise we'll leak.
(Assignee)

Comment 2

4 years ago
Created attachment 8475675 [details] [diff] [review]
Patch 1: Don't leak when output arrives while flushing MP4Reader

Ensure we don't leak if output arrives while we're flushing.
Attachment #8475675 - Flags: review?(ajones)
(Assignee)

Comment 3

4 years ago
Created attachment 8475676 [details] [diff] [review]
Patch 2: Make HasAudio work sooner in MP4Reader::ReadMetadata

Make MP4Reader::HasAudio() work earlier in ReadMetadata(), as we need it to tell whether we should create a PDM and audio decoder which we wrap with a EME Decryptor.
Attachment #8475676 - Flags: review?(ajones)
Attachment #8475675 - Flags: review?(ajones) → review+
Attachment #8475676 - Flags: review?(ajones) → review+

Comment 5

4 years ago
https://hg.mozilla.org/mozilla-central/rev/8c5f7a39a59a
https://hg.mozilla.org/mozilla-central/rev/a4200559a648
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.