Access MediaQueues from MediaDecoderStateMachine through an accessor

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

29 Branch
mozilla31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

It's convenient to access the MediaQueue's from the MediaDecoderStateMachine via accessor methods. This will enable us to move the MediaQueues from the Reader into the StateMachine (or wherever else for that matter) easily.

Moving the MediaQueues is required for supporting async Readers.
Posted patch PatchSplinter Review
Access MediaDecoderReader's MediaQueues from accessor functions inside the State Machine. This means we can add MediaQueues in the State Machine, which we'll need to do to adapt the current synchronous Readers to an async model.
Attachment #8413448 - Flags: review?(kinetik)
Attachment #8413448 - Flags: review?(kinetik) → review+
It looks like we lose const-correctness for several functions. Will it be brought back after moving MediaQueues to the StateMachine?
We can't use const anymore here because the MediaQueue's functions have to lock its monitor, and taking the monitor modifies it, breaking const. We'd have to make the MediaQueue's monitor mutable to retain const-ness here... I'm not sure if I like that or not, so I didn't do it.
Posted file const members.
The const modifier applies to MediaDecoderStateMachine instead of MediaQueue which is a member of the reader. I see no build error with this patch.
The next patch in my queue (from bug 979104) adds MediaQueues as members to the MediaDecoderStateMachine. I'm trying to land the patches in my queue that can land separately to reduce merge conflicts.
https://hg.mozilla.org/mozilla-central/rev/61b91c85a7aa
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Target Milestone: mozilla32 → mozilla31
(In reply to Chris Pearce (:cpearce) from comment #4)
> We can't use const anymore here because the MediaQueue's functions have to
> lock its monitor, and taking the monitor modifies it, breaking const. We'd
> have to make the MediaQueue's monitor mutable to retain const-ness here...
> I'm not sure if I like that or not, so I didn't do it.

I thought about this some more and discussed it with the guys in Auckland, and we're OK with making locks "mutable" so that non-modifying accessors can remain const. So we can put const back on these functions that I removed it from.
You need to log in before you can comment on or make changes to this bug.