RESOLVED FIXED 183961
Use special software encoder mode in case there is no VCP nor hardware encoder
https://bugs.webkit.org/show_bug.cgi?id=183961
Summary Use special software encoder mode in case there is no VCP nor hardware encoder
youenn fablet
Reported 2018-03-23 16:33:52 PDT
Use special software encoder mode in case there is no VCP not hardware encoder
Attachments
Patch (7.26 KB, patch)
2018-03-23 16:40 PDT, youenn fablet
no flags
Patch (7.49 KB, patch)
2018-03-23 17:02 PDT, youenn fablet
no flags
Patch for landing (7.45 KB, patch)
2018-04-09 12:39 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2018-03-23 16:40:25 PDT
youenn fablet
Comment 2 2018-03-23 17:02:51 PDT
Eric Carlson
Comment 3 2018-04-09 11:58:50 PDT
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.
youenn fablet
Comment 4 2018-04-09 12:19:13 PDT
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.
youenn fablet
Comment 5 2018-04-09 12:39:10 PDT
Created attachment 337526 [details] Patch for landing
WebKit Commit Bot
Comment 6 2018-04-09 14:10:00 PDT
Comment on attachment 337526 [details] Patch for landing Clearing flags on attachment: 337526 Committed r230451: <https://trac.webkit.org/changeset/230451>
WebKit Commit Bot
Comment 7 2018-04-09 14:10:01 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-04-09 14:10:23 PDT
Note You need to log in before you can comment on or make changes to this bug.