Skip to content

Commit

Permalink
[MediaStream] Address review comments after 270621@main
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=265378
rdar://118832787

Reviewed by Youenn Fablet.

* Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:
(WebCore::MediaStreamTrack::applyConstraints): Resolve or reject promise in a task.
(WebCore::MediaStreamTrack::trackEnded): Wrap long comment.

* Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:
(WebCore::AVVideoCaptureSource::resolvePendingPhotoRequest): Log an error message if there
is no producer.
(WebCore::AVVideoCaptureSource::rejectPendingPhotoRequest): Ditto.
(WebCore::AVVideoCaptureSource::captureOutputDidFinishProcessingPhoto): Return early if
capture fails.

* Source/WebCore/platform/mock/MockRealtimeVideoSource.h:
(WebCore::MockRealtimeVideoSource::DrawingState::DrawingState): Add `explicit`

* Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:
(WebKit::UserMediaCaptureManagerProxy::takePhoto):

Canonical link: https://commits.webkit.org/271155@main
  • Loading branch information
eric-carlson committed Nov 27, 2023
1 parent 46ea518 commit 0454dad
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 22 deletions.
21 changes: 12 additions & 9 deletions Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,14 +409,16 @@ void MediaStreamTrack::applyConstraints(const std::optional<MediaTrackConstraint
return;
}

m_private->applyConstraints(createMediaConstraints(constraints), [protectedThis = Ref { *this }, constraints, promise = WTFMove(promise)](auto&& error) mutable {
if (error) {
promise.rejectType<IDLInterface<OverconstrainedError>>(OverconstrainedError::create(WTFMove(error->badConstraint), WTFMove(error->message)));
return;
}

protectedThis->m_constraints = valueOrDefault(constraints);
promise.resolve();
m_private->applyConstraints(createMediaConstraints(constraints), [this, protectedThis = Ref { *this }, constraints, promise = WTFMove(promise)](auto&& error) mutable {
queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [protectedThis = WTFMove(protectedThis), error = WTFMove(error), constraints, promise = WTFMove(promise)]() mutable {
if (error) {
promise.rejectType<IDLInterface<OverconstrainedError>>(OverconstrainedError::create(WTFMove(error->badConstraint), WTFMove(error->message)));
return;
}

protectedThis->m_constraints = valueOrDefault(constraints);
promise.resolve();
});
});
}

Expand Down Expand Up @@ -613,7 +615,8 @@ void MediaStreamTrack::trackEnded(MediaStreamTrackPrivate&)
scriptExecutionContext()->addConsoleMessage(MessageSource::JS, MessageLevel::Error, "A MediaStreamTrack ended due to a capture failure"_s);

// http://w3c.github.io/mediacapture-main/#life-cycle
// When a MediaStreamTrack track ends for any reason other than the stop() method being invoked, the User Agent must queue a task that runs the following steps:
// When a MediaStreamTrack track ends for any reason other than the stop() method being invoked, the User Agent must
// queue a task that runs the following steps:
queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, muted = m_private->muted()] {
// 1. If the track's readyState attribute has the value ended already, then abort these steps.
if (!isAllowedToRunScript() || m_readyState == State::Ended)
Expand Down
19 changes: 11 additions & 8 deletions Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,10 @@ static bool isZoomSupported(const Vector<VideoPreset>& presets)
{
Locker lock { m_photoLock };

if (!m_photoProducer)
if (!m_photoProducer) {
ERROR_LOG_IF(loggerPtr(), LOGIDENTIFIER, "no photo producer");
return;
}

m_photoProducer->resolve(std::make_pair(WTFMove(data), mimeType));
m_photoProducer = nullptr;
Expand All @@ -551,8 +553,10 @@ static bool isZoomSupported(const Vector<VideoPreset>& presets)
{
Locker lock { m_photoLock };

if (!m_photoProducer)
if (!m_photoProducer) {
ERROR_LOG_IF(loggerPtr(), LOGIDENTIFIER, "no photo producer");
return;
}

m_photoProducer->reject(error);
m_photoProducer = nullptr;
Expand Down Expand Up @@ -1121,18 +1125,17 @@ static inline IntDegrees sensorOrientation(AVCaptureVideoOrientation videoOrient

void AVVideoCaptureSource::captureOutputDidFinishProcessingPhoto(RetainPtr<AVCapturePhotoOutput>, RetainPtr<AVCapturePhoto> photo, RetainPtr<NSError> error)
{
if (!error) {
NSData* data = [photo fileDataRepresentation];
resolvePendingPhotoRequest({ static_cast<const uint8_t*>(data.bytes), data.length }, "image/jpeg"_s);
} else
rejectPendingPhotoRequest("AVCapturePhotoOutput failed"_s);

if (error) {
rejectPendingPhotoRequest("AVCapturePhotoOutput failed"_s);
RunLoop::main().dispatch([this, protectedThis = Ref { *this }, logIdentifier = LOGIDENTIFIER, error = WTFMove(error) ] {
ASSERT(isMainThread());
ALWAYS_LOG_IF(loggerPtr(), logIdentifier, "failed: ", [error code], ", ", error.get());
});
return;
}

NSData* data = [photo fileDataRepresentation];
resolvePendingPhotoRequest({ static_cast<const uint8_t*>(data.bytes), data.length }, "image/jpeg"_s);
}

void AVVideoCaptureSource::captureSessionIsRunningDidChange(bool state)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/mock/MockRealtimeVideoSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class MockRealtimeVideoSource : public RealtimeVideoCaptureSource, private Orien

class DrawingState {
public:
DrawingState(float baseFontSize)
explicit DrawingState(float baseFontSize)
: m_baseFontSize(baseFontSize)
, m_bipBopFontSize(baseFontSize * 2.5)
, m_statsFontSize(baseFontSize * .5)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2017-2022 Apple Inc. All rights reserved.
* Copyright (C) 2017-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -640,9 +640,7 @@ void UserMediaCaptureManagerProxy::takePhoto(RealtimeMediaSourceIdentifier sourc
return;
}

proxy->takePhoto(WTFMove(settings))->whenSettled(RunLoop::main(), [handler = WTFMove(handler)] (auto&& result) mutable {
handler(WTFMove(result));
});
proxy->takePhoto(WTFMove(settings))->whenSettled(RunLoop::main(), WTFMove(handler));
}


Expand Down

0 comments on commit 0454dad

Please sign in to comment.