Skip to content

Commit cb02666

Browse files
committed
fix(video): Handle empty frame dimensions when screen grabs are denied.
If a video capture fails (e.g. gdigrab error), it may emit frames with 0x0 dimensions. `VideoFrame::toQImage` previously checked for `isValid()`, which incorrectly included 0x0 sizes, causing nullptr derefs. `isEmpty()` catches zero dimensions. Also, added nullptr checks in the internal frame storage logic.
1 parent 46aa07c commit cb02666

7 files changed

Lines changed: 111 additions & 13 deletions

File tree

cmake/Testing.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ if("EmojiOne" IN_LIST SMILEY_PACKS)
6363
auto_test(persistence smileypack "${SMILEY_RESOURCES}" "") # needs emojione
6464
endif()
6565
auto_test(video videomode "" "")
66+
auto_test(video videoframe "" "${LIBAVUTIL_LIBRARIES}")
6667
auto_test(widget filesform "" "")
6768
auto_test(widget/form/settings generalform "" "")
6869
auto_test(widget/tool identicon "" "")

src/video/videoframe.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,14 @@ void VideoFrame::releaseFrame()
279279
*/
280280
QImage VideoFrame::toQImage(QSize frameSize)
281281
{
282-
if (!frameSize.isValid()) {
282+
if (frameSize.isEmpty()) {
283283
frameSize = sourceDimensions.size();
284284
}
285285

286+
if (frameSize.isEmpty()) {
287+
return {};
288+
}
289+
286290
// Returns an empty constructed QImage in case of invalid generation
287291
auto [image, frameLocker] =
288292
toGenericObject(frameSize, AV_PIX_FMT_RGB24, false, [frameSize](AVFrame* const frame) {
@@ -312,6 +316,10 @@ std::pair<ToxYUVFrame, ReadWriteLocker> VideoFrame::toToxYUVFrame()
312316
{
313317
const QSize frameSize = sourceDimensions.size();
314318

319+
if (frameSize.isEmpty()) {
320+
return {ToxYUVFrame{}, ReadWriteLocker()};
321+
}
322+
315323
return toGenericObject(frameSize, AV_PIX_FMT_YUV420P, true, [frameSize](AVFrame* const frame) {
316324
// Converter function (constructs ToxAVFrame out of AVFrame*)
317325
return ToxYUVFrame{
@@ -592,6 +600,10 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor
592600
*/
593601
AVFrame* VideoFrame::storeAVFrame(AVFrame* frame, const QSize& dimensions, const int pixelFormat)
594602
{
603+
if (frame == nullptr) {
604+
return nullptr;
605+
}
606+
595607
const FrameBufferKey frameKey = getFrameKey(dimensions, pixelFormat, frame->linesize[0]);
596608

597609
// We check the presence of the frame in case of double-computation
@@ -700,8 +712,12 @@ VideoFrame::toGenericObject(const QSize& dimensions, int pixelFormat, bool requi
700712
*/
701713
{
702714
ReadWriteLocker frameWriteLock(&frameLock, ReadWriteLocker::WriteLock);
703-
return {objectConstructor(storeAVFrame(frame, dimensions, pixelFormat)),
704-
std::move(frameWriteLock)};
715+
AVFrame* storedFrame = storeAVFrame(frame, dimensions, pixelFormat);
716+
if (storedFrame == nullptr) {
717+
return {ReturnType{}, ReadWriteLocker()};
718+
}
719+
720+
return {objectConstructor(storedFrame), std::move(frameWriteLock)};
705721
}
706722
}
707723

test/BUILD.bazel

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,24 @@ UI_TESTS = ["loginscreen_test"]
9191
"model/exiftransform_test.cpp",
9292
"persistence/dbschema_test.cpp",
9393
"platform/stacktrace_test.cpp",
94+
"video/videoframe_test.cpp",
9495
"**/*_fuzz_test.cpp",
9596
],
9697
)]
9798

99+
qt_test(
100+
name = "videoframe_test",
101+
size = "small",
102+
src = "video/videoframe_test.cpp",
103+
copts = COPTS,
104+
mocopts = ["-Iqtox"],
105+
deps = [
106+
"//qtox/src",
107+
"@ffmpeg",
108+
"@qt//:qt_core",
109+
],
110+
)
111+
98112
cc_fuzz_test(
99113
name = "serialize_fuzz_test",
100114
size = "small",

test/chatlog/chatwidget_test.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
#include "src/chatlog/chatwidget.h"
66

7-
#include "src/chatlog/chatlinestorage.h"
87
#include "src/chatlog/documentcache.h"
9-
#include "src/chatlog/pixmapcache.h"
108
#include "src/core/icoreidhandler.h"
119
#include "src/model/ichatlog.h"
1210
#include "src/persistence/settings.h"

test/net/updateversion_test.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ void TestUpdateVersion::testTagToVersion()
3030

3131
void TestUpdateVersion::testIsUpdateAvailable()
3232
{
33-
Version v123{1, 2, 3};
34-
Version v124{1, 2, 4};
35-
Version v130{1, 3, 0};
36-
Version v200{2, 0, 0};
33+
const Version v123{1, 2, 3};
34+
const Version v124{1, 2, 4};
35+
const Version v130{1, 3, 0};
36+
const Version v200{2, 0, 0};
3737

3838
QVERIFY(isUpdateAvailable(v123, v124));
3939
QVERIFY(isUpdateAvailable(v123, v130));

test/video/videoframe_test.cpp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/* SPDX-License-Identifier: GPL-3.0-or-later
2+
* Copyright © 2026 The TokTok team.
3+
*/
4+
5+
#include "src/video/videoframe.h"
6+
7+
#include <QObject>
8+
#include <QtTest/QtTest>
9+
10+
extern "C"
11+
{
12+
#include <libavcodec/avcodec.h>
13+
#include <libavutil/imgutils.h>
14+
}
15+
16+
class TestVideoFrame : public QObject
17+
{
18+
Q_OBJECT
19+
20+
private slots:
21+
void testToQImageEmptySize();
22+
void testToQImageEmptySource();
23+
void testToToxYUVFrameEmptySource();
24+
};
25+
26+
void TestVideoFrame::testToQImageEmptySize()
27+
{
28+
AVFrame* frame = av_frame_alloc();
29+
frame->width = 100;
30+
frame->height = 100;
31+
frame->format = AV_PIX_FMT_YUV420P;
32+
av_image_alloc(frame->data, frame->linesize, 100, 100, AV_PIX_FMT_YUV420P, 1);
33+
34+
// VideoFrame takes ownership and will free the frame if we pass true
35+
auto videoFrame = VideoFrame::fromAVFrameUntracked(1, frame, true);
36+
37+
// This should not crash. Since QSize(0,0) is empty, it should fall back to source size (100x100)
38+
const QImage img = videoFrame->toQImage(QSize(0, 0));
39+
QCOMPARE(img.size(), QSize(100, 100));
40+
}
41+
42+
void TestVideoFrame::testToQImageEmptySource()
43+
{
44+
AVFrame* frame = av_frame_alloc();
45+
frame->width = 0;
46+
frame->height = 0;
47+
frame->format = AV_PIX_FMT_YUV420P;
48+
49+
auto videoFrame = VideoFrame::fromAVFrameUntracked(1, frame, true);
50+
51+
// Both requested size and source size are empty, should return null image
52+
const QImage img = videoFrame->toQImage(QSize(0, 0));
53+
QVERIFY(img.isNull());
54+
}
55+
56+
void TestVideoFrame::testToToxYUVFrameEmptySource()
57+
{
58+
AVFrame* frame = av_frame_alloc();
59+
frame->width = 0;
60+
frame->height = 0;
61+
frame->format = AV_PIX_FMT_YUV420P;
62+
63+
auto videoFrame = VideoFrame::fromAVFrameUntracked(1, frame, true);
64+
65+
auto [toxFrame, locker] = videoFrame->toToxYUVFrame();
66+
QVERIFY(!toxFrame.isValid());
67+
}
68+
69+
QTEST_GUILESS_MAIN(TestVideoFrame)
70+
#include "videoframe_test.moc"

test/widget/loginscreen_test.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,14 @@ private slots:
5353

5454
void TestLoginScreen::testLoginScreen()
5555
{
56-
// NOLINTNEXTLINE(misc-const-correctness)
57-
LoginScreen loginScreen(paths, style, themeColor, profileName);
56+
LoginScreen loginScreen(paths, style, themeColor, profileName); // NOLINT(misc-const-correctness)
5857

5958
COMPARE_GRAB(&loginScreen, "loginscreen_empty.png");
6059
}
6160

6261
void TestLoginScreen::testCreateProfile()
6362
{
64-
LoginScreen loginScreen(paths, style, themeColor, profileName);
63+
LoginScreen loginScreen(paths, style, themeColor, profileName); // NOLINT(misc-const-correctness)
6564

6665
bool created = false;
6766
QObject::connect(&loginScreen, &LoginScreen::createNewProfile, this,
@@ -78,7 +77,7 @@ void TestLoginScreen::testCreateProfile()
7877

7978
void TestLoginScreen::testCreateProfileBadPassword()
8079
{
81-
LoginScreen loginScreen(paths, style, themeColor, profileName);
80+
LoginScreen loginScreen(paths, style, themeColor, profileName); // NOLINT(misc-const-correctness)
8281

8382
bool created = false;
8483
connect(&loginScreen, &LoginScreen::createNewProfile, this, [&created]() { created = true; });

0 commit comments

Comments
 (0)