Bug 183961

Summary: Use special software encoder mode in case there is no VCP nor hardware encoder
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: 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 Flags
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2018-03-23 16:33:52 PDT
Use special software encoder mode in case there is no VCP not hardware encoder
Comment 1 youenn fablet 2018-03-23 16:40:25 PDT
Created attachment 336440 [details]
Patch
Comment 2 youenn fablet 2018-03-23 17:02:51 PDT
Created attachment 336444 [details]
Patch
Comment 3 Eric Carlson 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.
Comment 4 youenn fablet 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.
Comment 5 youenn fablet 2018-04-09 12:39:10 PDT
Created attachment 337526 [details]
Patch for landing
Comment 6 WebKit Commit Bot 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>
Comment 7 WebKit Commit Bot 2018-04-09 14:10:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-04-09 14:10:23 PDT
<rdar://problem/39292552>