Summary: | Use special software encoder mode in case there is no VCP nor hardware encoder | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson, webkit-bug-importer, youennf | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
youenn fablet
2018-03-23 16:33:52 PDT
Created attachment 336440 [details]
Patch
Created attachment 336444 [details]
Patch
Comment on attachment 336444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336444&action=review > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:674 > + CFTypeRef values[attributesSize] = {kCFBooleanTrue, ioSurfaceValue, pixelFormat}; Nit: declaring this immediately after "keys", and using the same one-line-per-value style, would make it easier to understand. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:678 > + ioSurfaceValue = nullptr; Nit: this is never accessed again so clearing it isn't necessary. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:682 > + pixelFormat = nullptr; Ditto. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:685 > + CFMutableDictionaryRef encoder_specs = CFDictionaryCreateMutable( Nit: you use camel case for the other local variables so this should probably as well. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:686 > + nullptr, 2, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); Nit: I don't think the line break makes this more readable. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:693 > + usage = nullptr; Nit: ditto the comment above about clearing. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:711 > + sourceAttributes = nullptr; Ditto. > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:715 > + encoder_specs = nullptr; Ditto. Comment on attachment 336444 [details]
Patch
We discussed with Eric and will keep clearing the variables even though that is not necessary, for consistency with the style of the file.
Will fix the other comments at land time.
Created attachment 337526 [details]
Patch for landing
Comment on attachment 337526 [details] Patch for landing Clearing flags on attachment: 337526 Committed r230451: <https://trac.webkit.org/changeset/230451> All reviewed patches have been landed. Closing bug. |