-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new encoder factory for UWP MF encoder #37
base: releases/m71
Are you sure you want to change the base?
Conversation
…in_sw_codecs=false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we able to move to one approach only? We are fine to move from deprecated solution and to use webrtc::VideoEncoderFactory
as a base class.
On the other hand, I've found this comment in WebRTC sources:
struct CodecInfo {
// |is_hardware_accelerated| is true if the encoders created by this factory
// of the given codec will use hardware support.
bool is_hardware_accelerated;
// |has_internal_source| is true if encoders created by this factory of the
// given codec will use internal camera sources, meaning that they don't
// require/expect frames to be delivered via webrtc::VideoEncoder::Encode.
// This flag is used as the internal_source parameter to
// webrtc::ViEExternalCodec::RegisterExternalSendCodec.
bool has_internal_source;
};
I searched the code and is_hardware_accelerated
and has_internal_source
are not used for codec selection. Not sure whether the flags are needed at all. MF based encoder uses HW resources if available on the platform.
I would suggest to delete old approach and to make a PR for webrtc::VideoEncoderFactory
base class. No need for two factories, anyway.
I've found out it's used in |
Would be nice to have a single factor with feature options. |
Alright, then should I set |
Yes, I agree that HW support should be always on for MF based encoder engine. But anyway we don't need two factories. I'm fine to move to the newer factory API and to keep the HW acceleration flag set to |
I've removed the old encoder factory and renamed the new one so it has the same name. |
The rationale behind this is the ability to use the UWP H264 encoder when building with
rtc_use_builtin_sw_codecs=false
because that flag disables someCreatePeerconnectionFactory
overloads that takecricket::WebRtcVideoEncoderFactory
(which is deprecated anyway) instead ofwebrtc::VideoEncoderFactory
.The naming isn't perfect but I thought as long as
cricket::WebRtcVideoEncoderFactory
is still used somewhere in M71 I should not delete it.