Bug 183961 - Use special software encoder mode in case there is no VCP nor hardware encoder
Summary: Use special software encoder mode in case there is no VCP nor hardware encoder
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
Keywords: InRadar
Depends on:
Reported: 2018-03-23 16:33 PDT by youenn fablet
Modified: 2018-04-09 14:10 PDT (History)
4 users (show)

See Also:

Patch (7.26 KB, patch)
2018-03-23 16:40 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (7.49 KB, patch)
2018-03-23 17:02 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (7.45 KB, patch)
2018-04-09 12:39 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
Comment 2 youenn fablet 2018-03-23 17:02:51 PDT
Created attachment 336444 [details]
Comment 3 Eric Carlson 2018-04-09 11:58:50 PDT
Comment on attachment 336444 [details]

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;


> 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;


> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/RTCVideoEncoderH264.mm:715
> +      encoder_specs = nullptr;

Comment 4 youenn fablet 2018-04-09 12:19:13 PDT
Comment on attachment 336444 [details]

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