From 52911b7060fc3341c4ab4b308be1e47f985e8576 Mon Sep 17 00:00:00 2001
From: Arjen Hiemstra <ahiemstra@heimr.nl>
Date: Thu, 20 Mar 2025 14:26:36 +0100
Subject: [PATCH] Change Encoder::applyEncodingPreference() to
 buildEncodingOptions()

As it turns out, FFmpeg does something to the pointers here that causes
the dictionary to never be properly used. This means we were never
applying the encoding options, as avcodec_open() was passed a nullptr
for options.

Fix this by changing the function to return an AVDictionary* instead of
trying to modify a passed-in pointer. This results in a proper dict
being returned, that can then be passed to avcodec_open().

The main result of this is that we now properly apply the encoding
options for VP9 encoding, which means we can now encode VP9 at realtime
speeds instead of it massively lagging behind.

BUG: 488896
---
 src/encoder.cpp            |  6 +++++-
 src/encoder_p.h            |  2 +-
 src/gifencoder.cpp         |  7 -------
 src/gifencoder_p.h         |  1 -
 src/h264vaapiencoder.cpp   | 10 +++++-----
 src/h264vaapiencoder_p.h   |  2 +-
 src/libopenh264encoder.cpp | 10 +++++-----
 src/libopenh264encoder_p.h |  2 +-
 src/libvpxencoder.cpp      | 10 ++++++----
 src/libvpxencoder_p.h      |  2 +-
 src/libvpxvp9encoder.cpp   | 10 ++++++----
 src/libvpxvp9encoder_p.h   |  2 +-
 src/libwebpencoder_p.h     |  1 -
 src/libx264encoder.cpp     | 11 ++++++-----
 src/libx264encoder_p.h     |  2 +-
 15 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/src/encoder.cpp b/src/encoder.cpp
index 41e6ac58..33617850 100644
--- a/src/encoder.cpp
+++ b/src/encoder.cpp
@@ -185,8 +185,10 @@ void Encoder::setEncodingPreference(PipeWireBaseEncodedStream::EncodingPreferenc
     m_encodingPreference = preference;
 }
 
-void Encoder::applyEncodingPreference(AVDictionary *options)
+AVDictionary *Encoder::buildEncodingOptions()
 {
+    AVDictionary *options = NULL;
+
     switch (m_encodingPreference) {
     case PipeWireBaseEncodedStream::EncodingPreference::NoPreference:
         av_dict_set(&options, "preset", "veryfast", 0);
@@ -205,6 +207,8 @@ void Encoder::applyEncodingPreference(AVDictionary *options)
         av_dict_set(&options, "preset", "veryfast", 0);
         break;
     }
+
+    return options;
 }
 
 SoftwareEncoder::SoftwareEncoder(PipeWireProduce *produce)
diff --git a/src/encoder_p.h b/src/encoder_p.h
index 1acd2548..48d3a9ea 100644
--- a/src/encoder_p.h
+++ b/src/encoder_p.h
@@ -102,6 +102,7 @@ public:
 
 protected:
     virtual int percentageToAbsoluteQuality(const std::optional<quint8> &quality) = 0;
+    virtual AVDictionary *buildEncodingOptions();
 
     PipeWireProduce *m_produce;
 
@@ -114,7 +115,6 @@ protected:
 
     std::optional<quint8> m_quality;
     PipeWireBaseEncodedStream::EncodingPreference m_encodingPreference;
-    virtual void applyEncodingPreference(AVDictionary *options);
 };
 
 /**
diff --git a/src/gifencoder.cpp b/src/gifencoder.cpp
index 4e24dea4..e2d9802b 100644
--- a/src/gifencoder.cpp
+++ b/src/gifencoder.cpp
@@ -54,9 +54,6 @@ bool GifEncoder::initialize(const QSize &size)
     m_avCodecContext->time_base = AVRational{1, 1000};
 
     AVDictionary *options = nullptr;
-
-    applyEncodingPreference(options);
-
     if (int result = avcodec_open2(m_avCodecContext, codec, &options); result < 0) {
         qCWarning(PIPEWIRERECORD_LOGGING) << "Could not open codec" << av_err2str(result);
         return false;
@@ -79,7 +76,3 @@ int GifEncoder::percentageToAbsoluteQuality([[maybe_unused]] const std::optional
 {
     return -1; // Not possible to set quality
 }
-
-void GifEncoder::applyEncodingPreference([[maybe_unused]] AVDictionary *options)
-{
-}
diff --git a/src/gifencoder_p.h b/src/gifencoder_p.h
index 9eceb0f2..5e2c0568 100644
--- a/src/gifencoder_p.h
+++ b/src/gifencoder_p.h
@@ -18,5 +18,4 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
 };
diff --git a/src/h264vaapiencoder.cpp b/src/h264vaapiencoder.cpp
index cf315f3c..26e7fba2 100644
--- a/src/h264vaapiencoder.cpp
+++ b/src/h264vaapiencoder.cpp
@@ -140,9 +140,7 @@ bool H264VAAPIEncoder::initialize(const QSize &size)
         break;
     }
 
-    AVDictionary *options = nullptr;
-    // av_dict_set_int(&options, "threads", qMin(16, QThread::idealThreadCount()), 0);
-    applyEncodingPreference(options);
+    AVDictionary *options = buildEncodingOptions();
 
     // Assign the right hardware context for encoding frames.
     // We rely on FFmpeg for creating the VAAPI hardware context as part of
@@ -168,11 +166,13 @@ int H264VAAPIEncoder::percentageToAbsoluteQuality(const std::optional<quint8> &q
     return std::max(1, int(MinQuality - (m_quality.value() / 100.0) * MinQuality));
 }
 
-void H264VAAPIEncoder::applyEncodingPreference(AVDictionary *options)
+AVDictionary *H264VAAPIEncoder::buildEncodingOptions()
 {
-    HardwareEncoder::applyEncodingPreference(options);
+    AVDictionary *options = HardwareEncoder::buildEncodingOptions();
     // Disable motion estimation, not great while dragging windows but speeds up encoding by an order of magnitude
     av_dict_set(&options, "flags", "+mv4", 0);
     // Disable in-loop filtering
     av_dict_set(&options, "-flags", "+loop", 0);
+
+    return options;
 }
diff --git a/src/h264vaapiencoder_p.h b/src/h264vaapiencoder_p.h
index 9a226508..3b02b335 100644
--- a/src/h264vaapiencoder_p.h
+++ b/src/h264vaapiencoder_p.h
@@ -20,7 +20,7 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
+    AVDictionary *buildEncodingOptions() override;
 
 private:
     H264Profile m_profile = H264Profile::Main;
diff --git a/src/libopenh264encoder.cpp b/src/libopenh264encoder.cpp
index 6d4c6a18..7f374d25 100644
--- a/src/libopenh264encoder.cpp
+++ b/src/libopenh264encoder.cpp
@@ -74,9 +74,7 @@ bool LibOpenH264Encoder::initialize(const QSize &size)
         break;
     }
 
-    AVDictionary *options = nullptr;
-    av_dict_set_int(&options, "threads", qMin(16, QThread::idealThreadCount()), 0);
-    applyEncodingPreference(options);
+    AVDictionary *options = buildEncodingOptions();
 
     if (int result = avcodec_open2(m_avCodecContext, codec, &options); result < 0) {
         qCWarning(PIPEWIRERECORD_LOGGING) << "Could not open codec" << av_err2str(result);
@@ -96,11 +94,13 @@ int LibOpenH264Encoder::percentageToAbsoluteQuality(const std::optional<quint8>
     return 51 - (m_quality.value() / 100.0) * 50;
 }
 
-void LibOpenH264Encoder::applyEncodingPreference(AVDictionary *options)
+AVDictionary *LibOpenH264Encoder::buildEncodingOptions()
 {
-    SoftwareEncoder::applyEncodingPreference(options);
+    AVDictionary *options = SoftwareEncoder::buildEncodingOptions();
     // Disable motion estimation, not great while dragging windows but speeds up encoding by an order of magnitude
     av_dict_set(&options, "flags", "+mv4", 0);
     // Disable in-loop filtering
     av_dict_set_int(&options, "loopfilter", 0, 0);
+
+    return options;
 }
diff --git a/src/libopenh264encoder_p.h b/src/libopenh264encoder_p.h
index fdacf147..64fb8848 100644
--- a/src/libopenh264encoder_p.h
+++ b/src/libopenh264encoder_p.h
@@ -21,7 +21,7 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
+    AVDictionary *buildEncodingOptions() override;
 
 private:
     H264Profile m_profile = H264Profile::Main;
diff --git a/src/libvpxencoder.cpp b/src/libvpxencoder.cpp
index d0cc9ea7..2d696d6e 100644
--- a/src/libvpxencoder.cpp
+++ b/src/libvpxencoder.cpp
@@ -57,8 +57,7 @@ bool LibVpxEncoder::initialize(const QSize &size)
         m_avCodecContext->global_quality = 35;
     }
 
-    AVDictionary *options = nullptr;
-    applyEncodingPreference(options);
+    AVDictionary *options = buildEncodingOptions();
 
     if (int result = avcodec_open2(m_avCodecContext, codec, &options); result < 0) {
         qCWarning(PIPEWIRERECORD_LOGGING) << "Could not open codec" << av_err2str(result);
@@ -78,9 +77,10 @@ int LibVpxEncoder::percentageToAbsoluteQuality(const std::optional<quint8> &qual
     return std::max(1, int(MinQuality - (m_quality.value() / 100.0) * MinQuality));
 }
 
-void LibVpxEncoder::applyEncodingPreference(AVDictionary *options)
+AVDictionary *LibVpxEncoder::buildEncodingOptions()
 {
-    av_dict_set_int(&options, "threads", qMin(16, QThread::idealThreadCount()), 0);
+    AVDictionary *options = SoftwareEncoder::buildEncodingOptions();
+
     av_dict_set(&options, "preset", "veryfast", 0);
     av_dict_set(&options, "tune-content", "screen", 0);
     av_dict_set(&options, "deadline", "realtime", 0);
@@ -91,4 +91,6 @@ void LibVpxEncoder::applyEncodingPreference(AVDictionary *options)
     // Disable in-loop filtering
     av_dict_set(&options, "-flags", "+loop", 0);
     av_dict_set(&options, "crf", "45", 0);
+
+    return options;
 }
diff --git a/src/libvpxencoder_p.h b/src/libvpxencoder_p.h
index 22fcae87..419117ae 100644
--- a/src/libvpxencoder_p.h
+++ b/src/libvpxencoder_p.h
@@ -20,5 +20,5 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
+    AVDictionary *buildEncodingOptions() override;
 };
diff --git a/src/libvpxvp9encoder.cpp b/src/libvpxvp9encoder.cpp
index d3ce94b0..0b630155 100644
--- a/src/libvpxvp9encoder.cpp
+++ b/src/libvpxvp9encoder.cpp
@@ -50,9 +50,7 @@ bool LibVpxVp9Encoder::initialize(const QSize &size)
     m_avCodecContext->pix_fmt = AV_PIX_FMT_YUV420P;
     m_avCodecContext->time_base = AVRational{1, 1000};
 
-    AVDictionary *options = nullptr;
-
-    applyEncodingPreference(options);
+    AVDictionary *options = buildEncodingOptions();
 
     const auto area = size.width() * size.height();
     // m_avCodecContext->framerate is not set, so we use m_produce->maxFramerate() instead.
@@ -89,8 +87,10 @@ int LibVpxVp9Encoder::percentageToAbsoluteQuality(const std::optional<quint8> &q
     return std::max(1, int(MinQuality - (m_quality.value() / 100.0) * MinQuality));
 }
 
-void LibVpxVp9Encoder::applyEncodingPreference(AVDictionary *options)
+AVDictionary *LibVpxVp9Encoder::buildEncodingOptions()
 {
+    AVDictionary *options = SoftwareEncoder::buildEncodingOptions();
+
     // We're probably capturing a screen
     av_dict_set(&options, "tune-content", "screen", 0);
 
@@ -119,4 +119,6 @@ void LibVpxVp9Encoder::applyEncodingPreference(AVDictionary *options)
     // This should make things faster, but it only seems to consume 100MB more RAM.
     // av_dict_set(&options, "row-mt", "1", 0);
     av_dict_set(&options, "frame-parallel", "1", 0);
+
+    return options;
 }
diff --git a/src/libvpxvp9encoder_p.h b/src/libvpxvp9encoder_p.h
index c2008273..863fda2d 100644
--- a/src/libvpxvp9encoder_p.h
+++ b/src/libvpxvp9encoder_p.h
@@ -21,5 +21,5 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
+    AVDictionary *buildEncodingOptions() override;
 };
diff --git a/src/libwebpencoder_p.h b/src/libwebpencoder_p.h
index a6d5782a..e4bfd125 100644
--- a/src/libwebpencoder_p.h
+++ b/src/libwebpencoder_p.h
@@ -17,5 +17,4 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
 };
diff --git a/src/libx264encoder.cpp b/src/libx264encoder.cpp
index 91011645..7d4f96b2 100644
--- a/src/libx264encoder.cpp
+++ b/src/libx264encoder.cpp
@@ -78,9 +78,7 @@ bool LibX264Encoder::initialize(const QSize &size)
         break;
     }
 
-    AVDictionary *options = nullptr;
-    av_dict_set_int(&options, "threads", qMin(16, QThread::idealThreadCount()), 0);
-    applyEncodingPreference(options);
+    AVDictionary *options = buildEncodingOptions();
 
     if (int result = avcodec_open2(m_avCodecContext, codec, &options); result < 0) {
         qCWarning(PIPEWIRERECORD_LOGGING) << "Could not open codec" << av_err2str(result);
@@ -100,11 +98,14 @@ int LibX264Encoder::percentageToAbsoluteQuality(const std::optional<quint8> &qua
     return std::max(1, int(MinQuality - (m_quality.value() / 100.0) * MinQuality));
 }
 
-void LibX264Encoder::applyEncodingPreference(AVDictionary *options)
+AVDictionary *LibX264Encoder::buildEncodingOptions()
 {
-    SoftwareEncoder::applyEncodingPreference(options);
+    AVDictionary *options = SoftwareEncoder::buildEncodingOptions();
+
     // Disable motion estimation, not great while dragging windows but speeds up encoding by an order of magnitude
     av_dict_set(&options, "flags", "+mv4", 0);
     // Disable in-loop filtering
     av_dict_set(&options, "-flags", "+loop", 0);
+
+    return options;
 }
diff --git a/src/libx264encoder_p.h b/src/libx264encoder_p.h
index c8b877c9..0a800758 100644
--- a/src/libx264encoder_p.h
+++ b/src/libx264encoder_p.h
@@ -20,7 +20,7 @@ public:
 
 protected:
     int percentageToAbsoluteQuality(const std::optional<quint8> &quality) override;
-    void applyEncodingPreference(AVDictionary *options) override;
+    AVDictionary *buildEncodingOptions() override;
 
 private:
     H264Profile m_profile = H264Profile::Main;
-- 
GitLab

