Skip to content

Commit

Permalink
Remove ParseSdpForH264ProfileLevelId defaulting introduced in https:/…
Browse files Browse the repository at this point in the history 
…/commits.webkit.org/266392@main

https://bugs.webkit.org/show_bug.cgi?id=260467
rdar://problem/114311260

Reviewed by Eric Carlson.

Before this patch, in case of bad or unsupported profile, we would downgrade to a default profile.
We are now reporting the error back from RTCVideoEncoderH264 to LibWebRTCCodecsProxy then to LibWebRTCCodecs and RemoteVideoCodecFactory.
This allows to notify WebCodecs JS that a particular profile is not supported.

Covered by added test.

* LayoutTests/http/wpt/webcodecs/H264-422-expected.txt: Added.
* LayoutTests/http/wpt/webcodecs/H264-422.html: Added.
* Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/h264_profile_level_id.cc:
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitEncoder.mm:
(-[WK_RTCLocalVideoH264H265Encoder initWithCodecInfo:]):
(webrtc::createLocalEncoder):
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.mm:
(-[RTCVideoEncoderH264 initWithCodecInfo:]):
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.h:
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.messages.in:
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:
(WebKit::LibWebRTCCodecsProxy::createEncoder):
(WebKit::LibWebRTCCodecsProxy::releaseEncoder):
(WebKit::LibWebRTCCodecsProxy::initializeEncoder):
(WebKit::LibWebRTCCodecsProxy::encodeFrame):
(WebKit::LibWebRTCCodecsProxy::flushEncoder):
(WebKit::LibWebRTCCodecsProxy::setEncodeRates):
(WebKit::LibWebRTCCodecsProxy::setSharedVideoFrameSemaphore):
(WebKit::LibWebRTCCodecsProxy::setSharedVideoFrameMemory):
* Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp:
(WebKit::RemoteVideoCodecFactory::createEncoder):
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
(WebKit::LibWebRTCCodecs::createEncoder):
(WebKit::LibWebRTCCodecs::createEncoderAndWaitUntilReady):
(WebKit::LibWebRTCCodecs::createEncoderInternal):
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h:

Canonical link: https://commits.webkit.org/267193@main
  • Loading branch information
@youennf
youennf committed Aug 23, 2023
1 parent 9853d19 commit 9743e04
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 37 deletions.
3 changes: 3 additions & 0 deletions LayoutTests/http/wpt/webcodecs/H264-422-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS Test H264 4.2.2 is not supported

20 changes: 20 additions & 0 deletions LayoutTests/http/wpt/webcodecs/H264-422.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html>
<header>
<script src='/resources/testharness.js'></script>
<script src='/resources/testharnessreport.js'></script>
</header>
<body>
<script>
promise_test(async t => {
const result = await VideoEncoder.isConfigSupported({
codec: "avc1.7A1032",
width: 1920,
height: 1280,
avc: { format: "avc" },
});
assert_false(result.supported);
}, 'Test H264 4.2.2 is not supported');
</script>
</body>
</html>
2 changes: 1 addition & 1 deletion Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/h264_profile_level_id.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ absl::optional<H264ProfileLevelId> ParseSdpForH264ProfileLevelId(
// FIXME: We might want to return std::nullopt and propagate an error in case of bad parsing.
return (profile_level_id_it == params.end())
? defaultProfileLevelId()
: ParseH264ProfileLevelId(profile_level_id_it->second.c_str()).value_or(defaultProfileLevelId());
: ParseH264ProfileLevelId(profile_level_id_it->second.c_str());
}
#else
absl::optional<H264ProfileLevelId> ParseSdpForH264ProfileLevelId(
Expand Down
8 changes: 7 additions & 1 deletion Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitEncoder.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,11 @@ - (instancetype)initWithCodecInfo:(RTCVideoCodecInfo*)codecInfo {
if (self = [super init]) {
if ([codecInfo.name isEqualToString:@"H265"])
m_h265Encoder = [[RTCVideoEncoderH265 alloc] initWithCodecInfo:codecInfo];
else
else {
m_h264Encoder = [[RTCVideoEncoderH264 alloc] initWithCodecInfo:codecInfo];
if (!m_h264Encoder)
return nil;
}
}
return self;
}
Expand Down Expand Up @@ -344,6 +347,9 @@ void encoderVideoTaskComplete(void* callback, webrtc::VideoCodecType codecType,
auto *codecInfo = [[RTCVideoCodecInfo alloc] initWithNativeSdpVideoFormat: format];
auto *encoder = [[WK_RTCLocalVideoH264H265Encoder alloc] initWithCodecInfo:codecInfo];

if (!encoder)
return nullptr;

[encoder setCallback:^BOOL(RTCEncodedImage *_Nonnull frame, id<RTCCodecSpecificInfo> _Nonnull codecSpecificInfo, RTCRtpFragmentationHeader * _Nullable header) {
EncodedImage encodedImage = [frame nativeEncodedImage];

Expand Down
6 changes: 5 additions & 1 deletion ...ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,11 @@ - (instancetype)initWithCodecInfo:(RTCVideoCodecInfo *)codecInfo {
_packetizationMode = RTCH264PacketizationModeNonInterleaved;
_profile_level_id =
webrtc::ParseSdpForH264ProfileLevelId([codecInfo nativeSdpVideoFormat].parameters);
_useBaseline = !_profile_level_id || ![(__bridge NSString *)ExtractProfile(*_profile_level_id) containsString: @"High"];
if (!_profile_level_id) {
return nil;
}

_useBaseline = ![(__bridge NSString *)ExtractProfile(*_profile_level_id) containsString: @"High"];
#if ENABLE_VCP_FOR_H264_BASELINE
_useVCP = true;
#else
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class LibWebRTCCodecsProxy final : public IPC::WorkQueueMessageReceiver {
void decodeFrame(VideoDecoderIdentifier, int64_t timeStamp, const IPC::DataReference&);
void setFrameSize(VideoDecoderIdentifier, uint16_t width, uint16_t height);

void createEncoder(VideoEncoderIdentifier, VideoCodecType, const Vector<std::pair<String, String>>&, bool useLowLatency, bool useAnnexB);
void createEncoder(VideoEncoderIdentifier, VideoCodecType, const Vector<std::pair<String, String>>&, bool useLowLatency, bool useAnnexB, CompletionHandler<void(bool)>&&);
void releaseEncoder(VideoEncoderIdentifier);
void initializeEncoder(VideoEncoderIdentifier, uint16_t width, uint16_t height, unsigned startBitrate, unsigned maxBitrate, unsigned minBitrate, uint32_t maxFramerate);
void encodeFrame(VideoEncoderIdentifier, SharedVideoFrame&&, int64_t timeStamp, std::optional<uint64_t> duration, bool shouldEncodeAsKeyFrame);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.messages.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ messages -> LibWebRTCCodecsProxy NotRefCounted {
DecodeFrame(WebKit::VideoDecoderIdentifier id, int64_t timeStamp, IPC::DataReference data)
SetFrameSize(WebKit::VideoDecoderIdentifier id, uint16_t width, uint16_t height)

CreateEncoder(WebKit::VideoEncoderIdentifier id, enum:uint8_t WebKit::VideoCodecType codecType, Vector<std::pair<String, String>> parameters, bool useLowLatency, bool useAnnexB);
CreateEncoder(WebKit::VideoEncoderIdentifier id, enum:uint8_t WebKit::VideoCodecType codecType, Vector<std::pair<String, String>> parameters, bool useLowLatency, bool useAnnexB) -> (bool success);
ReleaseEncoder(WebKit::VideoEncoderIdentifier id)
InitializeEncoder(WebKit::VideoEncoderIdentifier id, uint16_t width, uint16_t height, unsigned startBitrate, unsigned maxBitrate, unsigned minBitrate, uint32_t maxFramerate)
EncodeFrame(WebKit::VideoEncoderIdentifier id, struct WebKit::SharedVideoFrame buffer, int64_t timeStamp, std::optional<uint64_t> duration, bool shouldEncodeAsKeyFrame)
Expand Down
35 changes: 15 additions & 20 deletions Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@
task(iterator->value);
}

void LibWebRTCCodecsProxy::createEncoder(VideoEncoderIdentifier identifier, VideoCodecType codecType, const Vector<std::pair<String, String>>& parameters, bool useLowLatency, bool useAnnexB)
void LibWebRTCCodecsProxy::createEncoder(VideoEncoderIdentifier identifier, VideoCodecType codecType, const Vector<std::pair<String, String>>& parameters, bool useLowLatency, bool useAnnexB, CompletionHandler<void(bool)>&& callback)
{
assertIsCurrent(workQueue());
std::map<std::string, std::string> rtcParameters;
Expand All @@ -253,20 +253,25 @@
});

auto* encoder = webrtc::createLocalEncoder(webrtc::SdpVideoFormat { codecType == VideoCodecType::H264 ? "H264" : "H265", rtcParameters }, useAnnexB, newFrameBlock.get(), newConfigurationBlock.get());
if (!encoder) {
callback(false);
return;
}

webrtc::setLocalEncoderLowLatency(encoder, useLowLatency);
auto result = m_encoders.add(identifier, Encoder { encoder, makeUnique<SharedVideoFrameReader>(Ref { m_videoFrameObjectHeap }, m_resourceOwner) });
ASSERT_UNUSED(result, result.isNewEntry || IPC::isTestingIPC());
m_hasEncodersOrDecoders = true;
callback(true);
}

void LibWebRTCCodecsProxy::releaseEncoder(VideoEncoderIdentifier identifier)
{
assertIsCurrent(workQueue());
auto encoder = m_encoders.take(identifier);
if (!encoder.webrtcEncoder) {
ASSERT_IS_TESTING_IPC();
if (!encoder.webrtcEncoder)
return;
}

webrtc::releaseLocalEncoder(encoder.webrtcEncoder);
m_hasEncodersOrDecoders = !m_encoders.isEmpty() || !m_decoders.isEmpty();
}
Expand All @@ -275,10 +280,9 @@
{
assertIsCurrent(workQueue());
auto* encoder = findEncoder(identifier);
if (!encoder) {
ASSERT_IS_TESTING_IPC();
if (!encoder)
return;
}

webrtc::initializeLocalEncoder(encoder->webrtcEncoder, width, height, startBitrate, maxBitrate, minBitrate, maxFramerate);
}

Expand Down Expand Up @@ -311,7 +315,6 @@
assertIsCurrent(workQueue());
auto* encoder = findEncoder(identifier);
if (!encoder) {
ASSERT_IS_TESTING_IPC();
// Make sure to read RemoteVideoFrameReadReference to prevent memory leaks.
if (std::holds_alternative<RemoteVideoFrameReadReference>(sharedVideoFrame.buffer))
m_videoFrameObjectHeap->get(WTFMove(std::get<RemoteVideoFrameReadReference>(sharedVideoFrame.buffer)));
Expand Down Expand Up @@ -340,10 +343,8 @@
{
assertIsCurrent(workQueue());
auto* encoder = findEncoder(identifier);
if (!encoder) {
ASSERT_IS_TESTING_IPC();
if (!encoder)
return;
}

webrtc::flushLocalEncoder(encoder->webrtcEncoder);
m_connection->send(Messages::LibWebRTCCodecs::FlushEncoderCompleted { identifier }, 0);
Expand All @@ -353,10 +354,8 @@
{
assertIsCurrent(workQueue());
auto* encoder = findEncoder(identifier);
if (!encoder) {
ASSERT_IS_TESTING_IPC();
if (!encoder)
return;
}

webrtc::setLocalEncoderRates(encoder->webrtcEncoder, bitRate, frameRate);
}
Expand All @@ -365,10 +364,8 @@
{
assertIsCurrent(workQueue());
auto* encoder = findEncoder(identifier);
if (!encoder) {
ASSERT_IS_TESTING_IPC();
if (!encoder)
return;
}

encoder->frameReader->setSemaphore(WTFMove(semaphore));
}
Expand All @@ -377,10 +374,8 @@
{
assertIsCurrent(workQueue());
auto* encoder = findEncoder(identifier);
if (!encoder) {
ASSERT_IS_TESTING_IPC();
if (!encoder)
return;
}

encoder->frameReader->setSharedMemory(WTFMove(handle));
}
Expand Down
10 changes: 8 additions & 2 deletions Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,15 @@ void RemoteVideoCodecFactory::createEncoder(const String& codec, const WebCore::
}
}

WebProcess::singleton().libWebRTCCodecs().createEncoderAndWaitUntilReady(*type, parameters, config.isRealtime, config.useAnnexB, [config, createCallback = WTFMove(createCallback), descriptionCallback = WTFMove(descriptionCallback), outputCallback = WTFMove(outputCallback), postTaskCallback = WTFMove(postTaskCallback)](auto& internalEncoder) mutable {
WebProcess::singleton().libWebRTCCodecs().createEncoderAndWaitUntilReady(*type, parameters, config.isRealtime, config.useAnnexB, [config, createCallback = WTFMove(createCallback), descriptionCallback = WTFMove(descriptionCallback), outputCallback = WTFMove(outputCallback), postTaskCallback = WTFMove(postTaskCallback)](auto* internalEncoder) mutable {
if (!internalEncoder) {
postTaskCallback([createCallback = WTFMove(createCallback)]() mutable {
createCallback(makeUnexpected("Encoder creation failed"_s));
});
return;
}
auto callbacks = RemoteVideoEncoderCallbacks::create(WTFMove(descriptionCallback), WTFMove(outputCallback), WTFMove(postTaskCallback));
UniqueRef<VideoEncoder> encoder = makeUniqueRef<RemoteVideoEncoder>(internalEncoder, config, callbacks.copyRef());
UniqueRef<VideoEncoder> encoder = makeUniqueRef<RemoteVideoEncoder>(*internalEncoder, config, callbacks.copyRef());
callbacks->postTask([createCallback = WTFMove(createCallback), encoder = WTFMove(encoder)]() mutable {
createCallback(WTFMove(encoder));
});
Expand Down
23 changes: 15 additions & 8 deletions Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,15 +505,15 @@ static inline webrtc::VideoCodecType toWebRTCCodecType(VideoCodecType type)

LibWebRTCCodecs::Encoder* LibWebRTCCodecs::createEncoder(VideoCodecType type, const std::map<std::string, std::string>& parameters)
{
return createEncoderInternal(type, parameters, true, true, [](auto&) { });
return createEncoderInternal(type, parameters, true, true, [](auto*) { });
}

void LibWebRTCCodecs::createEncoderAndWaitUntilReady(VideoCodecType type, const std::map<std::string, std::string>& parameters, bool isRealtime, bool useAnnexB, Function<void(Encoder&)>&& callback)
void LibWebRTCCodecs::createEncoderAndWaitUntilReady(VideoCodecType type, const std::map<std::string, std::string>& parameters, bool isRealtime, bool useAnnexB, Function<void(Encoder*)>&& callback)
{
createEncoderInternal(type, parameters, isRealtime, useAnnexB, WTFMove(callback));
}

LibWebRTCCodecs::Encoder* LibWebRTCCodecs::createEncoderInternal(VideoCodecType type, const std::map<std::string, std::string>& formatParameters, bool isRealtime, bool useAnnexB, Function<void(Encoder&)>&& callback)
LibWebRTCCodecs::Encoder* LibWebRTCCodecs::createEncoderInternal(VideoCodecType type, const std::map<std::string, std::string>& formatParameters, bool isRealtime, bool useAnnexB, Function<void(Encoder*)>&& callback)
{
auto encoder = makeUnique<Encoder>();
auto* result = encoder.get();
Expand All @@ -529,25 +529,32 @@ LibWebRTCCodecs::Encoder* LibWebRTCCodecs::createEncoderInternal(VideoCodecType
ensureGPUProcessConnectionAndDispatchToThread([this, encoder = WTFMove(encoder), parameters = WTFMove(parameters), callback = WTFMove(callback)]() mutable {
assertIsCurrent(workQueue());

auto* encoderPointer = encoder.get();

auto connection = [&]() -> Ref<IPC::Connection> {
Locker locker { m_connectionLock };
return *m_connection;
}();

{
Locker locker { m_encodersConnectionLock };
connection->send(Messages::LibWebRTCCodecsProxy::CreateEncoder { encoder->identifier, encoder->type, parameters, encoder->isRealtime, encoder->useAnnexB }, 0);
connection->sendWithAsyncReply(Messages::LibWebRTCCodecsProxy::CreateEncoder { encoder->identifier, encoder->type, parameters, encoder->isRealtime, encoder->useAnnexB }, [identifier = encoder->identifier, callback = WTFMove(callback)](bool result) mutable {
WebProcess::singleton().libWebRTCCodecs().m_queue->dispatch([identifier, result, callback = WTFMove(callback)]() mutable {
if (!result) {
callback(nullptr);
return;
}

auto& codecs = WebProcess::singleton().libWebRTCCodecs();
assertIsCurrent(codecs.workQueue());
callback(codecs.m_encoders.get(identifier));
});
}, 0);
setEncoderConnection(*encoder, connection.ptr());
}

encoder->parameters = WTFMove(parameters);
auto encoderIdentifier = encoder->identifier;
ASSERT(!m_encoders.contains(encoderIdentifier));
m_encoders.add(encoderIdentifier, WTFMove(encoder));

callback(*encoderPointer);
});
return result;
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class LibWebRTCCodecs : public IPC::WorkQueueMessageReceiver, public GPUProcessC
};

Encoder* createEncoder(VideoCodecType, const std::map<std::string, std::string>&);
void createEncoderAndWaitUntilReady(VideoCodecType, const std::map<std::string, std::string>&, bool isRealtime, bool useAnnexB, Function<void(Encoder&)>&&);
void createEncoderAndWaitUntilReady(VideoCodecType, const std::map<std::string, std::string>&, bool isRealtime, bool useAnnexB, Function<void(Encoder*)>&&);
int32_t releaseEncoder(Encoder&);
int32_t initializeEncoder(Encoder&, uint16_t width, uint16_t height, unsigned startBitrate, unsigned maxBitrate, unsigned minBitrate, uint32_t maxFramerate);
int32_t encodeFrame(Encoder&, const WebCore::VideoFrame&, int64_t timestamp, std::optional<uint64_t> duration, bool shouldEncodeAsKeyFrame);
Expand Down Expand Up @@ -188,7 +188,7 @@ class LibWebRTCCodecs : public IPC::WorkQueueMessageReceiver, public GPUProcessC
WorkQueue& workQueue() const { return m_queue; }

Decoder* createDecoderInternal(VideoCodecType, Function<void(Decoder&)>&&);
Encoder* createEncoderInternal(VideoCodecType, const std::map<std::string, std::string>&, bool isRealtime, bool useAnnexB, Function<void(Encoder&)>&&);
Encoder* createEncoderInternal(VideoCodecType, const std::map<std::string, std::string>&, bool isRealtime, bool useAnnexB, Function<void(Encoder*)>&&);
template<typename Frame> int32_t encodeFrameInternal(Encoder&, const Frame&, bool shouldEncodeAsKeyFrame, WebCore::VideoFrameRotation, MediaTime, int64_t timestamp, std::optional<uint64_t> duration);

private:
Expand Down

0 comments on commit 9743e04

Please sign in to comment.