Fix encoding for codecs with defined fixed frame rates

For such codecs selected time base should match
1 / frame_rate, it was set just frame_rate in the previous impl.
Also, in the patch we set an adjusted frame rate to the codec context.

Change-Id: I33ced3fdd908dbc016b2638387939ad32bd6f030
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Artem Dyomin <artem.dyomin@qt.io>
Reviewed-by: Jøger Hansegård <joger.hansegard@qt.io>
(cherry picked from commit fa1fcce7dae95753f21586a47e6a37ab77d5c773)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 116 insertions, 28 deletions

diff --git a/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils.cpp b/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils.cpp
index d24c4644c..2f53412d9 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils.cpp
+++ b/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils.cpp
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 
 #include "qffmpegvideoencoderutils_p.h"
+#include "private/qmultimediautils_p.h"
 
 extern "C" {
 #include <libavutil/pixdesc.h>
@@ -142,6 +143,72 @@ const AVCodec *findSwEncoder(AVCodecID codecID, AVPixelFormat sourceSWFormat)
     });
 }
 
+AVRational adjustFrameRate(const AVRational *supportedRates, qreal requestedRate)
+{
+    qreal diff = std::numeric_limits<qreal>::max();
+
+    auto getDiff = [requestedRate](qreal currentRate) {
+        return qMax(requestedRate, currentRate) / qMin(requestedRate, currentRate);
+
+        // Using just a liniar delta is also possible, but
+        // relative comparison should work better
+        // return qAbs(currentRate - requestedRate);
+    };
+
+    if (supportedRates) {
+        const AVRational *result = nullptr;
+        for (auto rate = supportedRates; rate->num && rate->den; ++rate) {
+            const qreal currentDiff = getDiff(qreal(rate->num) / rate->den);
+
+            if (currentDiff < diff) {
+                diff = currentDiff;
+                result = supportedRates;
+            }
+        }
+
+        if (result)
+            return *result;
+    }
+
+    const auto [num, den] = qRealToFraction(requestedRate);
+    return { num, den };
+}
+
+AVRational adjustFrameTimeBase(const AVRational *supportedRates, AVRational frameRate)
+{
+    // TODO: user-specified frame rate might be required.
+    if (supportedRates) {
+        auto hasFrameRate = [&]() {
+            for (auto rate = supportedRates; rate->num && rate->den; ++rate)
+                if (rate->den == frameRate.den && rate->num == frameRate.num)
+                    return true;
+
+            return false;
+        };
+
+        Q_ASSERT(hasFrameRate());
+
+        return { frameRate.den, frameRate.num };
+    }
+
+    constexpr int TimeScaleFactor = 1000; // Allows not to follow fixed rate
+    return { frameRate.den, frameRate.num * TimeScaleFactor };
+}
+
+QSize adjustVideoResolution(const AVCodec *codec, QSize requestedResolution)
+{
+#ifdef Q_OS_WINDOWS
+    // TODO: investigate, there might be more encoders not supporting odd resolution
+    if (strcmp(codec->name, "h264_mf") == 0) {
+        auto makeEven = [](int size) { return size & ~1; };
+        return QSize(makeEven(requestedResolution.width()), makeEven(requestedResolution.height()));
+    }
+#else
+    Q_UNUSED(codec);
+#endif
+    return requestedResolution;
+}
+
 } // namespace QFFmpeg
 
 QT_END_NAMESPACE
diff --git a/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils_p.h b/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils_p.h
index ab6ae5c38..3a16a7de3 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils_p.h
+++ b/src/plugins/multimedia/ffmpeg/qffmpegvideoencoderutils_p.h
@@ -32,6 +32,31 @@ std::pair<const AVCodec *, std::unique_ptr<HWAccel>> findHwEncoder(AVCodecID cod
 
 const AVCodec *findSwEncoder(AVCodecID codecID, AVPixelFormat sourceSWFormat);
 
+/**
+ * @brief adjustFrameRate get a rational frame rate be requested qreal rate.
+ *        If the codec supports fixed frame rate (non-null supportedRates),
+ *        the function selects the most suitable one,
+ *        otherwise just makes AVRational from qreal.
+ */
+AVRational adjustFrameRate(const AVRational *supportedRates, qreal requestedRate);
+
+/**
+ * @brief adjustFrameTimeBase gets adjusted timebase by a list of supported frame rates
+ *        and an already adjusted frame rate.
+ *
+ *        Timebase is the fundamental unit of time (in seconds) in terms
+ *        of which frame timestamps are represented.
+ *        For fixed-fps content (non-null supportedRates),
+ *        timebase should be 1/framerate.
+ *
+ *        For more information, see AVStream::time_base and AVCodecContext::time_base.
+ *
+ *        The adjusted time base is supposed to be set to stream and codec context.
+ */
+AVRational adjustFrameTimeBase(const AVRational *supportedRates, AVRational frameRate);
+
+QSize adjustVideoResolution(const AVCodec *codec, QSize requestedResolution);
+
 } // namespace QFFmpeg
 
 QT_END_NAMESPACE
diff --git a/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp b/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp
index 6ef0b25bc..bbd7f9407 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp
+++ b/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp
@@ -85,6 +85,23 @@ bool VideoFrameEncoder::initCodec()
     }
 #endif
 
+    auto fixedResolution = adjustVideoResolution(m_codec, m_settings.videoResolution());
+    if (resolution != fixedResolution) {
+        qCDebug(qLcVideoFrameEncoder) << "Fix odd video resolution for codec" << m_codec->name
+                                      << ":" << resolution << "->" << fixedResolution;
+
+        m_settings.setVideoResolution(fixedResolution);
+    }
+
+    if (m_codec->supported_framerates && qLcVideoFrameEncoder().isEnabled(QtDebugMsg))
+        for (auto rate = m_codec->supported_framerates; rate->num && rate->den; ++rate)
+            qCDebug(qLcVideoFrameEncoder)
+                    << "supported frame rate:" << rate->num << "/" << rate->den;
+
+    m_codecFrameRate = adjustFrameRate(m_codec->supported_framerates, m_settings.videoFrameRate());
+    qCDebug(qLcVideoFrameEncoder) << "Adjusted frame rate:" << m_codecFrameRate.num << "/"
+                                  << m_codecFrameRate.den;
+
     return true;
 }
 
@@ -139,32 +156,10 @@ bool QFFmpeg::VideoFrameEncoder::initCodecContext(AVFormatContext *formatContext
     m_stream->codecpar->width = resolution.width();
     m_stream->codecpar->height = resolution.height();
     m_stream->codecpar->sample_aspect_ratio = AVRational{ 1, 1 };
-    float requestedRate = m_settings.videoFrameRate();
-    constexpr int TimeScaleFactor = 1000; // Allows not to follow fixed rate
-    m_stream->time_base = AVRational{ 1, static_cast<int>(requestedRate * TimeScaleFactor) };
-
-    float delta = 1e10;
-    if (m_codec->supported_framerates) {
-        // codec only supports fixed frame rates
-        auto *best = m_codec->supported_framerates;
-        qCDebug(qLcVideoFrameEncoder) << "Finding fixed rate:";
-        for (auto *f = m_codec->supported_framerates; f->num != 0; f++) {
-            auto maybeRate = toFloat(*f);
-            if (!maybeRate)
-                continue;
-            float d = qAbs(*maybeRate - requestedRate);
-            qCDebug(qLcVideoFrameEncoder) << "    " << f->num << f->den << d;
-            if (d < delta) {
-                best = f;
-                delta = d;
-            }
-        }
-        qCDebug(qLcVideoFrameEncoder) << "Fixed frame rate required. Requested:" << requestedRate << "Using:" << best->num << "/" << best->den;
-        m_stream->time_base = *best;
-        requestedRate = toFloat(*best).value_or(0.f);
-    }
 
     Q_ASSERT(m_codec);
+
+    m_stream->time_base = adjustFrameTimeBase(m_codec->supported_framerates, m_codecFrameRate);
     m_codecContext.reset(avcodec_alloc_context3(m_codec));
     if (!m_codecContext) {
         qWarning() << "Could not allocate codec context";
@@ -173,10 +168,10 @@ bool QFFmpeg::VideoFrameEncoder::initCodecContext(AVFormatContext *formatContext
 
     avcodec_parameters_to_context(m_codecContext.get(), m_stream->codecpar);
     m_codecContext->time_base = m_stream->time_base;
-    qCDebug(qLcVideoFrameEncoder) << "requesting time base" << m_codecContext->time_base.num
+    qCDebug(qLcVideoFrameEncoder) << "codecContext time base" << m_codecContext->time_base.num
                                   << m_codecContext->time_base.den;
-    auto [num, den] = qRealToFraction(requestedRate);
-    m_codecContext->framerate = { num, den };
+
+    m_codecContext->framerate = m_codecFrameRate;
     m_codecContext->pix_fmt = m_targetFormat;
     m_codecContext->width = resolution.width();
     m_codecContext->height = resolution.height();
@@ -209,7 +204,6 @@ bool VideoFrameEncoder::open()
     }
     qCDebug(qLcVideoFrameEncoder) << "video codec opened" << res << "time base"
                                   << m_codecContext->time_base.num << m_codecContext->time_base.den;
-    m_stream->time_base = m_stream->time_base;
     return true;
 }
 
diff --git a/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder_p.h b/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder_p.h
index a977e13cd..d81247088 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder_p.h
+++ b/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder_p.h
@@ -73,6 +73,8 @@ private:
     bool m_downloadFromHW = false;
     bool m_uploadToHW = false;
 
+    AVRational m_codecFrameRate = { 0, 1 };
+
     int64_t m_prevPacketDts = AV_NOPTS_VALUE;
     int64_t m_packetDtsOffset = 0;
 };