Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Media recorder produces empty chunks
https://bugs.webkit.org/show_bug.cgi?id=257016
rdar://109705910

Reviewed by Eric Carlson.

We were skipping video frame generation when needing preparation for display.
This works fine as long as we receive the canvasDisplayBufferPrepared notification.
This notification only happens if the document is observing canvas changes.
If that is not the case, we schedule a capture canvas, which will trigger a video frame generation and will register the document as a canvas change observer.

Making a side fix in LayoutTests/webrtc/routines.js so that computeFrameRate works fine on browsers without internals.
This is necessary for LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas.html in particular.

* LayoutTests/http/wpt/mediarecorder/record-context-created-late-expected.txt: Added.
* LayoutTests/http/wpt/mediarecorder/record-context-created-late.html: Added.
* LayoutTests/webrtc/routines.js:
* Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp:
(WebCore::CanvasCaptureMediaStreamTrack::Source::canvasChanged):
* Source/WebCore/html/CanvasBase.cpp:
(WebCore::CanvasBase::hasObserver const):
* Source/WebCore/html/CanvasBase.h:

Canonical link: https://commits.webkit.org/264478@main
  • Loading branch information
youennf committed May 24, 2023
1 parent 03cda63 commit 712b8a9
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 2 deletions.
@@ -0,0 +1,3 @@

PASS Verify late canvas context creation does not break recording

@@ -0,0 +1,32 @@
<!doctype html>
<html>
<head>
<title>MediaRecorder context created late</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
<canvas id="canvas" width="500" height="500"></canvas>
<script>
promise_test(async (t) => {
const stream = canvas.captureStream(25);
const mediaRecorder = new MediaRecorder(stream)
mediaRecorder.start(10);

setTimeout(() => {
const ctx = canvas.getContext('2d');
ctx.fillStyle = 'green';
ctx.fillRect(10, 10, 150, 100);
}, 100);

return new Promise((resolve, reject) => {
mediaRecorder.ondataavailable = (event) => {
if (event.data.size > 0)
resolve();
};
setTimeout(() => reject("no blob with data"), 5000);
});
}, "Verify late canvas context creation does not break recording");
</script>
</body>
</html>
2 changes: 1 addition & 1 deletion LayoutTests/webrtc/routines.js
Expand Up @@ -263,7 +263,7 @@ function getReceivedTrackStats(connection)
return connection.getStats().then((report) => {
var stats;
report.forEach((statItem) => {
if (statItem.type === "track") {
if (statItem.type === "inbound-rtp") {
stats = statItem;
}
});
Expand Down
Expand Up @@ -156,7 +156,7 @@ void CanvasCaptureMediaStreamTrack::Source::canvasChanged(CanvasBase& canvas, co
{
ASSERT_UNUSED(canvas, m_canvas == &canvas);
#if ENABLE(WEBGL)
if (m_canvas->renderingContext() && m_canvas->renderingContext()->needsPreparationForDisplay())
if (m_canvas->renderingContext() && m_canvas->renderingContext()->needsPreparationForDisplay() && m_canvas->hasObserver(m_canvas->document()))
return;
#endif
scheduleCaptureCanvas();
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/html/CanvasBase.cpp
Expand Up @@ -195,6 +195,11 @@ void CanvasBase::removeObserver(CanvasObserver& observer)
InspectorInstrumentation::didChangeCSSCanvasClientNodes(*this);
}

bool CanvasBase::hasObserver(CanvasObserver& observer) const
{
return m_observers.contains(observer);
}

void CanvasBase::notifyObserversCanvasChanged(const std::optional<FloatRect>& rect)
{
for (auto& observer : m_observers)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/html/CanvasBase.h
Expand Up @@ -98,6 +98,7 @@ class CanvasBase {

void addObserver(CanvasObserver&);
void removeObserver(CanvasObserver&);
bool hasObserver(CanvasObserver&) const;
void notifyObserversCanvasChanged(const std::optional<FloatRect>&);
void notifyObserversCanvasResized();
void notifyObserversCanvasDestroyed(); // Must be called in destruction before clearing m_context.
Expand Down

0 comments on commit 712b8a9

Please sign in to comment.