Summary: | Use CMClock as a timing source for PlatformClock where available. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric.carlson | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Jer Noble
2012-02-06 11:07:54 PST
Created attachment 125671 [details]
Patch
Created attachment 125672 [details]
Patch
Comment on attachment 125672 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125672&action=review > Source/WebCore/platform/mac/PlatformClockCM.mm:48 > +static const int32_t DefaultTimeScale = 1000; Please include a comment explaining why 1000 is the best magic timescale. > Source/WebCore/platform/mac/PlatformClockCM.mm:57 > + CMAudioDeviceClockCreate(kCFAllocatorDefault, NULL, &rawClockPtr); > + RetainPtr<CMClockRef> clock(AdoptCF, rawClockPtr); > + initializeWithTimingSource(clock.get()); Is the temporary RetainPtr necessary? (In reply to comment #3) > (From update of attachment 125672 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125672&action=review > > > Source/WebCore/platform/mac/PlatformClockCM.mm:48 > > +static const int32_t DefaultTimeScale = 1000; > > Please include a comment explaining why 1000 is the best magic timescale. Sure thing. > > Source/WebCore/platform/mac/PlatformClockCM.mm:57 > > + CMAudioDeviceClockCreate(kCFAllocatorDefault, NULL, &rawClockPtr); > > + RetainPtr<CMClockRef> clock(AdoptCF, rawClockPtr); > > + initializeWithTimingSource(clock.get()); > > Is the temporary RetainPtr necessary? Not strictly, no. But the RetainPtr takes care of null-checking before calling CFRelease, and does so even if an exception is thrown. And I've been yelled at* for not using RetainPtrs for temporary variables before. ;) *figuratively Committed r106978: <http://trac.webkit.org/changeset/106978> |