Fix a few memory leaks

- avcodec_open2 doesn't take ownership of options
- fix not deleted surface in tests

Change-Id: Ia410792b27af4ef5ccaf1c6774344d3b6571952a
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Lars Knoll <lars@knoll.priv.no>
(cherry picked from commit 5e36c05dac20a997c5be7a8445daf78758fb275b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

5 files changed, 35 insertions, 20 deletions

diff --git a/src/plugins/multimedia/ffmpeg/playbackengine/qffmpegcodec.cpp b/src/plugins/multimedia/ffmpeg/playbackengine/qffmpegcodec.cpp
index 6a3b139f9..67d47cad8 100644
--- a/src/plugins/multimedia/ffmpeg/playbackengine/qffmpegcodec.cpp
+++ b/src/plugins/multimedia/ffmpeg/playbackengine/qffmpegcodec.cpp
@@ -63,10 +63,11 @@ QMaybe<Codec> Codec::create(AVStream *stream)
     context->get_format = QFFmpeg::getFormat;
 
     /* Init the decoder, with reference counting and threading */
-    AVDictionary *opts = nullptr;
-    av_dict_set(&opts, "refcounted_frames", "1", 0);
-    av_dict_set(&opts, "threads", "auto", 0);
-    ret = avcodec_open2(context.get(), decoder, &opts);
+    AVDictionaryHolder opts;
+    av_dict_set(opts, "refcounted_frames", "1", 0);
+    av_dict_set(opts, "threads", "auto", 0);
+
+    ret = avcodec_open2(context.get(), decoder, opts);
     if (ret < 0)
         return "Failed to open FFmpeg codec context " + err2str(ret);
 
diff --git a/src/plugins/multimedia/ffmpeg/qffmpeg_p.h b/src/plugins/multimedia/ffmpeg/qffmpeg_p.h
index 6dab5e882..44fd66cf2 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpeg_p.h
+++ b/src/plugins/multimedia/ffmpeg/qffmpeg_p.h
@@ -78,6 +78,19 @@ inline void getAVFrameTime(AVFrame &frame, int64_t &pts, AVRational &timeBase)
 #endif
 }
 
+struct AVDictionaryHolder
+{
+    AVDictionary *opts = nullptr;
+
+    operator AVDictionary **() { return &opts; }
+
+    ~AVDictionaryHolder()
+    {
+        if (opts)
+            av_dict_free(&opts);
+    }
+};
+
 template<typename FunctionType, FunctionType F>
 struct AVDeleter
 {
diff --git a/src/plugins/multimedia/ffmpeg/qffmpegencoder.cpp b/src/plugins/multimedia/ffmpeg/qffmpegencoder.cpp
index 3e3ea2670..276582977 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpegencoder.cpp
+++ b/src/plugins/multimedia/ffmpeg/qffmpegencoder.cpp
@@ -312,10 +312,10 @@ void AudioEncoder::open()
 
     avcodec_parameters_to_context(codec, stream->codecpar);
 
-    AVDictionary *opts = nullptr;
-    applyAudioEncoderOptions(settings, avCodec->name, codec, &opts);
+    AVDictionaryHolder opts;
+    applyAudioEncoderOptions(settings, avCodec->name, codec, opts);
 
-    int res = avcodec_open2(codec, avCodec, &opts);
+    int res = avcodec_open2(codec, avCodec, opts);
     qCDebug(qLcFFmpegEncoder) << "audio codec opened" << res;
     qCDebug(qLcFFmpegEncoder) << "audio codec params: fmt=" << codec->sample_fmt << "rate=" << codec->sample_rate;
 
diff --git a/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp b/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp
index ba7ead150..1f227073a 100644
--- a/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp
+++ b/src/plugins/multimedia/ffmpeg/qffmpegvideoframeencoder.cpp
@@ -198,9 +198,9 @@ bool VideoFrameEncoder::open()
         qWarning() << "Cannot open null VideoFrameEncoder";
         return false;
     }
-    AVDictionary *opts = nullptr;
-    applyVideoEncoderOptions(d->settings, d->codec->name, d->codecContext.get(), &opts);
-    int res = avcodec_open2(d->codecContext.get(), d->codec, &opts);
+    AVDictionaryHolder opts;
+    applyVideoEncoderOptions(d->settings, d->codec->name, d->codecContext.get(), opts);
+    int res = avcodec_open2(d->codecContext.get(), d->codec, opts);
     if (res < 0) {
         d->codecContext.reset();
         qWarning() << "Couldn't open codec for writing" << err2str(res);
diff --git a/tests/auto/integration/qmediaplayerbackend/tst_qmediaplayerbackend.cpp b/tests/auto/integration/qmediaplayerbackend/tst_qmediaplayerbackend.cpp
index 27c15c3c3..4e0e1695b 100644
--- a/tests/auto/integration/qmediaplayerbackend/tst_qmediaplayerbackend.cpp
+++ b/tests/auto/integration/qmediaplayerbackend/tst_qmediaplayerbackend.cpp
@@ -855,13 +855,13 @@ void tst_QMediaPlayerBackend::seekPauseSeek()
 
     QSignalSpy positionSpy(&player, SIGNAL(positionChanged(qint64)));
 
-    TestVideoSink *surface = new TestVideoSink;
-    player.setVideoOutput(surface);
+    TestVideoSink surface;
+    player.setVideoOutput(&surface);
 
     player.setSource(localVideoFile);
     QCOMPARE(player.playbackState(), QMediaPlayer::StoppedState);
     QTRY_COMPARE(player.mediaStatus(), QMediaPlayer::LoadedMedia);
-    QVERIFY(surface->m_frameList.isEmpty()); // frame must not appear until we call pause() or play()
+    QVERIFY(surface.m_frameList.isEmpty()); // frame must not appear until we call pause() or play()
 
     positionSpy.clear();
     qint64 position = 7000;
@@ -870,18 +870,19 @@ void tst_QMediaPlayerBackend::seekPauseSeek()
     QTRY_COMPARE(player.position(), position);
     QCOMPARE(player.playbackState(), QMediaPlayer::StoppedState);
     QTest::qWait(250); // wait a bit to ensure the frame is not rendered
-    QVERIFY(surface->m_frameList.isEmpty()); // still no frame, we must call pause() or play() to see a frame
+    QVERIFY(surface.m_frameList
+                    .isEmpty()); // still no frame, we must call pause() or play() to see a frame
 
     player.pause();
     QTRY_COMPARE(player.playbackState(), QMediaPlayer::PausedState); // it might take some time for the operation to be completed
-    QTRY_VERIFY(!surface->m_frameList.isEmpty()); // we must see a frame at position 7000 here
+    QTRY_VERIFY(!surface.m_frameList.isEmpty()); // we must see a frame at position 7000 here
 
     // Make sure that the frame has a timestamp before testing - not all backends provides this
-    if (!surface->m_frameList.back().isValid() || surface->m_frameList.back().startTime() < 0)
+    if (!surface.m_frameList.back().isValid() || surface.m_frameList.back().startTime() < 0)
         QSKIP("No timestamp");
 
     {
-        QVideoFrame frame = surface->m_frameList.back();
+        QVideoFrame frame = surface.m_frameList.back();
         const qint64 elapsed = (frame.startTime() / 1000) - position; // frame.startTime() is microsecond, position is milliseconds.
         QVERIFY2(qAbs(elapsed) < (qint64)500, QByteArray::number(elapsed).constData());
         QCOMPARE(frame.width(), 160);
@@ -895,16 +896,16 @@ void tst_QMediaPlayerBackend::seekPauseSeek()
         QVERIFY(qBlue(image.pixel(0, 0)) < 20);
     }
 
-    surface->m_frameList.clear();
+    surface.m_frameList.clear();
     positionSpy.clear();
     position = 12000;
     player.setPosition(position);
     QTRY_VERIFY(!positionSpy.isEmpty() && qAbs(player.position() - position) < (qint64)500);
     QCOMPARE(player.playbackState(), QMediaPlayer::PausedState);
-    QTRY_VERIFY(!surface->m_frameList.isEmpty());
+    QTRY_VERIFY(!surface.m_frameList.isEmpty());
 
     {
-        QVideoFrame frame = surface->m_frameList.back();
+        QVideoFrame frame = surface.m_frameList.back();
         const qint64 elapsed = (frame.startTime() / 1000) - position;
         QVERIFY2(qAbs(elapsed) < (qint64)500, QByteArray::number(elapsed).constData());
         QCOMPARE(frame.width(), 160);