Closed Bug 1227797 Opened 6 years ago Closed 6 years ago

Use MediaEventSource to publish playback events for MDSM

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

This help reduce bi-directional dependency between MediaDecoder and MDSM.
Bug 1227797 - Use MediaEventSource to publish playback events for MDSM. r=jya.
Attachment #8691675 - Flags: review?(jyavenard)
Comment on attachment 8691675 [details]
MozReview Request: Bug 1227797 - Use MediaEventSource to publish playback events for MDSM. r=jya.

https://reviewboard.mozilla.org/r/26165/#review23865

r=me ; however I have reservations about it as it looks a bit messy to me.

Couldn't we use instead a single MediaEventSource that receive an enum for each type of event to send: something like:
enum MediaEventType {
  kPlayBackStart,
  kPlayBackStop,
  ...
  
would look nicer and easier to extend than having to duplicates the number of MediaEventSource and all the methods related to it
Attachment #8691675 - Flags: review?(jyavenard) → review+
Ah, yes. I've been thinking about that. Since OnSeekingStart unlike others takes one argument, I wonder should I just leave it alone and merge others into one event source or use some kind of downcast tricks to merge them all.
(In reply to JW Wang [:jwwang] from comment #4)
> Ah, yes. I've been thinking about that. Since OnSeekingStart unlike others
> takes one argument, I wonder should I just leave it alone and merge others
> into one event source or use some kind of downcast tricks to merge them all.

I think so yes... (merging the others)
Thanks. I will file another bug to do that.
Blocks: 1228923
https://hg.mozilla.org/mozilla-central/rev/52fd8e1b0844
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.