Bug 200774 – [Cocoa] Adopt -preventDisplaySleepForVideoPlayback

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[Cocoa] Adopt -preventDisplaySleepForVideoPlayback

[Jer Noble]

<rdar://problem/54321071>

[Jer Noble]

Created attachment 376391 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 376391 [details]
Patch

Clearing flags on attachment: 376391

Committed r248747: <https://trac.webkit.org/changeset/248747>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 376391 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376391&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:70
> +@interface AVSampleBufferDisplayLayer (WebCorePrivate)
> +@property (assign, nonatomic) BOOL preventDisplaySleepForVideoPlayback;
> +@end

I’d like us to make it a habit to put these kinds of things into SPI headers whenever possible.

[Jer Noble]

(In reply to Darin Adler from comment #5)
> Comment on attachment 376391 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=376391&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:70
> > +@interface AVSampleBufferDisplayLayer (WebCorePrivate)
> > +@property (assign, nonatomic) BOOL preventDisplaySleepForVideoPlayback;
> > +@end
> 
> I’d like us to make it a habit to put these kinds of things into SPI headers
> whenever possible.

Understood. The SPI declarations for AVSampleBufferDisplayLayer in AVFoundationSPI.h are ... complicated, and I was trying to make the simplest patch possible to avoid any merge problems.

[Darin Adler]

(In reply to Jer Noble from comment #6)
> Understood. The SPI declarations for AVSampleBufferDisplayLayer in
> AVFoundationSPI.h are ... complicated, and I was trying to make the simplest
> patch possible to avoid any merge problems.

Fine with me for the short term. Not the right thing for the longer term, I think.

[Jer Noble]

(In reply to Darin Adler from comment #7)
> Fine with me for the short term. Not the right thing for the longer term, I
> think.

Filed bug# 200790 to track cleaning this all up.

[Jer Noble]

Darin was correct, I was wrong, and because I tried to do the most expedient fix instead of the most correct fix, a misspelling ("preventDisplaySleep" vs. "prevent_s_DisplaySleep") got into the build, negating the benefit of this patch. Fixing.

[Jer Noble]

Reopening to attach new patch.

[Jer Noble]

Created attachment 376725 [details]
Patch

[EWS Watchlist]

Attachment 376725 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/mac/AVFoundationSPI.h:310:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 376725 [details]
Patch

Clearing flags on attachment: 376725

Committed r248886: <https://trac.webkit.org/changeset/248886>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.