Skip to content

Commit

Permalink
[GStreamer][WebCodecs] New tests http/wpt/webcodecs/h264-encoder-defa…
Browse files Browse the repository at this point in the history
…ult-config trigger critical warnings

https://bugs.webkit.org/show_bug.cgi?id=265301

Reviewed by Xabier Rodriguez-Calvar.

There was a confusion in kbps/bps conversion, 1 kbps is 1000 bps, not 1024 bps... We also now check
the bitrate is positive and non-null before setting it on the encoder.

* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/platform/graphics/gstreamer/VideoEncoderGStreamer.cpp:
(WebCore::GStreamerInternalVideoEncoder::initialize):
* Source/WebCore/platform/gstreamer/VideoEncoderPrivateGStreamer.cpp:
* Source/WebCore/platform/mediastream/gstreamer/RealtimeOutgoingVideoSourceGStreamer.cpp:
(WebCore::RealtimeOutgoingVideoSourceGStreamer::updateStats):
(WebCore::RealtimeOutgoingVideoSourceGStreamer::setParameters):
(WebCore::RealtimeOutgoingVideoSourceGStreamer::fillEncodingParameters):

Canonical link: https://commits.webkit.org/271099@main
  • Loading branch information
philn committed Nov 24, 2023
1 parent a83fbf4 commit a20dfce
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 9 deletions.
2 changes: 0 additions & 2 deletions LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1208,8 +1208,6 @@ fast/mediastream/play-newly-added-audio-track.html [ Pass Crash ]
webkit.org/b/264663 media/media-source/media-source-webm-configuration-framerate.html [ Crash Pass ]

webkit.org/b/264665 http/wpt/webcodecs/videoFrame-webglCanvasImageSource.html [ Failure ]
webkit.org/b/264894 http/wpt/webcodecs/h264-encoder-default-config.https.any.html [ Failure ]
webkit.org/b/264894 http/wpt/webcodecs/h264-encoder-default-config.https.any.worker.html [ Failure ]

webkit.org/b/264666 imported/w3c/web-platform-tests/encrypted-media/clearkey-generate-request-disallowed-input.https.html [ Failure ]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ String GStreamerInternalVideoEncoder::initialize(const String& codecName, const
if (!videoEncoderSetFormat(WEBKIT_VIDEO_ENCODER(m_harness->element()), WTFMove(encoderCaps)))
return "Unable to set encoder format"_s;

if (config.bitRate)
g_object_set(m_harness->element(), "bitrate", static_cast<uint32_t>(config.bitRate / 1024), nullptr);
if (config.bitRate > 1000)
g_object_set(m_harness->element(), "bitrate", static_cast<uint32_t>(config.bitRate / 1000), nullptr);
m_isInitialized = true;
return emptyString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ using namespace WebCore;
GST_DEBUG_CATEGORY(video_encoder_debug);
#define GST_CAT_DEFAULT video_encoder_debug

#define KBIT_TO_BIT 1024
#define KBIT_TO_BIT 1000

// FIXME: Make this configurable at runtime?
#define NUMBER_OF_THREADS 4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ void RealtimeOutgoingVideoSourceGStreamer::updateStats(GstBuffer*)
if (m_encoder) {
uint32_t bitrate;
g_object_get(m_encoder.get(), "bitrate", &bitrate, nullptr);
gst_structure_set(m_stats.get(), "bitrate", G_TYPE_DOUBLE, static_cast<double>(bitrate * 1024), nullptr);
gst_structure_set(m_stats.get(), "bitrate", G_TYPE_DOUBLE, static_cast<double>(bitrate * 1000), nullptr);
}

gst_structure_set(m_stats.get(), "frames-sent", G_TYPE_UINT64, framesSent, "frames-encoded", G_TYPE_UINT64, framesSent, nullptr);
Expand Down Expand Up @@ -348,7 +348,8 @@ void RealtimeOutgoingVideoSourceGStreamer::setParameters(GUniquePtr<GstStructure
gst_structure_get(structure, "max-bitrate", G_TYPE_ULONG, &maxBitrate, nullptr);

// maxBitrate is expessed in bits/s but the encoder property is in Kbit/s.
g_object_set(m_encoder.get(), "bitrate", static_cast<unsigned>(maxBitrate / 1024), nullptr);
if (maxBitrate >= 1000)
g_object_set(m_encoder.get(), "bitrate", static_cast<uint32_t>(maxBitrate / 1000), nullptr);
}

void RealtimeOutgoingVideoSourceGStreamer::fillEncodingParameters(const GUniquePtr<GstStructure>& encodingParameters)
Expand All @@ -368,11 +369,11 @@ void RealtimeOutgoingVideoSourceGStreamer::fillEncodingParameters(const GUniqueP
gst_structure_set(encodingParameters.get(), "max-framerate", G_TYPE_DOUBLE, maxFrameRate, nullptr);
}

unsigned long maxBitrate = 2048 * 1024;
unsigned long maxBitrate = 2048 * 1000;
if (m_encoder) {
uint32_t bitrate;
g_object_get(m_encoder.get(), "bitrate", &bitrate, nullptr);
maxBitrate = bitrate * 1024;
maxBitrate = bitrate * 1000;
}

gst_structure_set(encodingParameters.get(), "max-bitrate", G_TYPE_ULONG, maxBitrate, nullptr);
Expand Down

0 comments on commit a20dfce

Please sign in to comment.