Skip to content

Commit 5dc8946

Browse files
committed
Improvements after review
1 parent 8d86820 commit 5dc8946

File tree

8 files changed

+62
-39
lines changed

8 files changed

+62
-39
lines changed

include/utils/MemoryTracker.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,14 @@ void objectDeleter(T* ptr, const QString& subComponent, const QString& typeName)
5353
// The object's thread is not running, which is a potential issue.
5454
// Log a critical error as this indicates a problem in the shutdown sequence.
5555
// This should be an *extremely rare* fallback and indicates a bug in the thread shutdown sequence.
56-
qCritical().noquote() << QString("|%1| QObject<%2> could not be deleted. Owning thread is not running.").arg(subComponent, typeName);
56+
qCritical().noquote() << QString("|%1| QObject<%2> could not be deleted. Owning thread is not running. This may cause a memory leak. Please check thread shutdown sequence and object ownership.")
57+
.arg(subComponent, typeName);
5758

59+
// [MEMORYTRACKER] CRITICAL: This code path indicates a potential memory leak, as the QObject cannot be safely deleted.
60+
// To aid debugging, ensure that all threads are properly shut down before object destruction.
61+
// Consider adding more diagnostics here, such as emitting a signal or logging additional context (e.g., stack trace, object address).
5862
//qCDebug(memory_objects).noquote() << QString("|%1| QObject<%2> object's owning thread is not running. Deleted immediately (thread not running)").arg(subComponent, typeName);
59-
//direct delete will require friendship, to be added (to Logger)
63+
//direct delete will require friendship, to be added (to Logger)
6064
//delete ptr;
6165
}
6266
}

include/utils/Profiler.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class Profiler
4242
private:
4343
static void initLogger();
4444

45-
QSharedPointer<Logger> _logger;
45+
static QSharedPointer<Logger> _logger;
4646
const char* _file;
4747
const char* _func;
4848
unsigned int _line;

libsrc/api/JsonInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ QJsonArray JsonInfo::discoverScreenInputs(const QJsonObject& params) const
715715
discoverGrabber<DDAGrabber>(screenInputs, params);
716716
#endif
717717

718-
#ifdef ENABLE_DRM && !defined(ENABLE_AMLOGIC)
718+
#if defined(ENABLE_DRM) && !defined(ENABLE_AMLOGIC)
719719
discoverGrabber<DRMFrameGrabber>(screenInputs, params);
720720
#endif
721721

libsrc/grabber/amlogic/AmlogicGrabber.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ bool AmlogicGrabber::isGbmSupported(bool logMsg) const
7171
{
7272
// Check for the existence of gbm_create_device, a core GBM function, within libdrm.so
7373

74-
QString libName = "libMali";
74+
QString libName = "libMali";
7575
QString lib = libName + ".so";
7676

7777
// 1. Attempt to open the library.

libsrc/grabber/drm/DRMFrameGrabber.cpp

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -224,58 +224,63 @@ bool DRMFrameGrabber::setWidthHeight(int width, int height)
224224
return false;
225225
}
226226

227-
static bool processLinearFramebuffer(int deviceFd,
228-
const drmModeFB2* framebuffer,
229-
int w,
230-
int h,
231-
PixelFormat pixelFormat,
232-
ImageResampler const& imageResampler,
233-
Logger* log,
234-
Image<ColorRgb>& image)
227+
struct LinearFramebufferParams
228+
{
229+
int deviceFd;
230+
const drmModeFB2* framebuffer;
231+
int w;
232+
int h;
233+
PixelFormat pixelFormat;
234+
const ImageResampler& imageResampler;
235+
QSharedPointer<Logger> log;
236+
Image<ColorRgb>& image;
237+
};
238+
239+
static bool processLinearFramebuffer(const LinearFramebufferParams& params)
235240
{
236241
int size = 0;
237242
int lineLength = 0;
238243
int fb_dmafd = 0;
239244

240-
if (pixelFormat == PixelFormat::I420 || pixelFormat == PixelFormat::NV12
245+
if (params.pixelFormat == PixelFormat::I420 || params.pixelFormat == PixelFormat::NV12
241246
#ifdef DRM_FORMAT_NV21
242-
|| pixelFormat == PixelFormat::NV21
247+
|| params.pixelFormat == PixelFormat::NV21
243248
#endif
244249
)
245250
{
246-
size = (w * h * 3) / 2;
247-
lineLength = w;
251+
size = (params.w * params.h * 3) / 2;
252+
lineLength = params.w;
248253
}
249254
#ifdef DRM_FORMAT_P030
250-
else if (pixelFormat == PixelFormat::P030)
255+
else if (params.pixelFormat == PixelFormat::P030)
251256
{
252-
size = (w * h * 2) + (DIV_ROUND_UP(w, 2) * DIV_ROUND_UP(h, 2) * 4); // Y16 + UV32 per 2px
253-
lineLength = w * 2; // 16bpp luma
257+
size = (params.w * params.h * 2) + (DIV_ROUND_UP(params.w, 2) * DIV_ROUND_UP(params.h, 2) * 4); // Y16 + UV32 per 2px
258+
lineLength = params.w * 2; // 16bpp luma
254259
}
255260
#endif
256-
else if (pixelFormat == PixelFormat::RGB32 || pixelFormat == PixelFormat::BGR32)
261+
else if (params.pixelFormat == PixelFormat::RGB32 || params.pixelFormat == PixelFormat::BGR32)
257262
{
258-
size = w * h * 4;
259-
lineLength = w * 4;
263+
size = params.w * params.h * 4;
264+
lineLength = params.w * 4;
260265
}
261266

262-
int ret = drmPrimeHandleToFD(deviceFd, framebuffer->handles[0], O_RDONLY, &fb_dmafd);
267+
int ret = drmPrimeHandleToFD(params.deviceFd, params.framebuffer->handles[0], O_RDONLY, &fb_dmafd);
263268
if (ret != 0)
264269
{
265-
Error(log, "drmPrimeHandleToFD failed (handle=%u): %s", framebuffer->handles[0], strerror(errno));
270+
Error(params.log, "drmPrimeHandleToFD failed (handle=%u): %s", params.framebuffer->handles[0], strerror(errno));
266271
return false;
267272
}
268273

269274
auto* mmapFrameBuffer = (uint8_t*)mmap(nullptr, size, PROT_READ, MAP_SHARED, fb_dmafd, 0);
270275
if (mmapFrameBuffer != MAP_FAILED)
271276
{
272-
imageResampler.processImage(mmapFrameBuffer, w, h, lineLength, pixelFormat, image);
277+
params.imageResampler.processImage(mmapFrameBuffer, params.w, params.h, lineLength, params.pixelFormat, params.image);
273278
munmap(mmapFrameBuffer, size);
274279
close(fb_dmafd);
275280
return true;
276281
}
277282

278-
Error(log, "Format: %s failed. Error: %s", QSTRING_CSTR(getDrmFormat(framebuffer->pixel_format)), strerror(errno));
283+
Error(params.log, "Format: %s failed. Error: %s", QSTRING_CSTR(getDrmFormat(params.framebuffer->pixel_format)), strerror(errno));
279284
close(fb_dmafd);
280285
return false;
281286
}
@@ -598,7 +603,17 @@ int DRMFrameGrabber::grabFrame(Image<ColorRgb> &image)
598603
// Linear modifier path
599604
if (_pixelFormat != PixelFormat::NO_CHANGE && modifier == DRM_FORMAT_MOD_LINEAR)
600605
{
601-
if (processLinearFramebuffer(_deviceFd, framebuffer, w, h, _pixelFormat, _imageResampler, _log.data(), image))
606+
LinearFramebufferParams params{
607+
_deviceFd,
608+
framebuffer,
609+
w,
610+
h,
611+
_pixelFormat,
612+
_imageResampler,
613+
_log,
614+
image
615+
};
616+
if (processLinearFramebuffer(params))
602617
{
603618
newImage = true;
604619
}

libsrc/grabber/video/v4l2/V4L2Grabber.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,17 +1556,17 @@ void V4L2Grabber::enumVideoCaptureDevices()
15561556
if (close(fd) < 0) continue;
15571557

15581558
QFile devNameFile(dev+"/name");
1559-
if (!devNameFile.exists() || !devNameFile.open(QFile::ReadOnly))
1560-
{
1561-
Error( _log, "Device file '%s' cannot be opened.", QSTRING_CSTR(devNameFile.fileName()) );
1562-
}
1563-
else
1559+
if (devNameFile.exists() && devNameFile.open(QFile::ReadOnly))
15641560
{
15651561
devName = devNameFile.readLine();
15661562
devName = devName.trimmed();
15671563
properties.name = devName;
15681564
devNameFile.close();
15691565
}
1566+
else
1567+
{
1568+
Error(_log, "Device file '%s' cannot be opened.", QSTRING_CSTR(devNameFile.fileName()));
1569+
}
15701570

15711571
_deviceProperties.insert("/dev/"+it.fileName(), properties);
15721572
}

libsrc/leddevice/dev_net/LedDeviceWled.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ LedDeviceWled::LedDeviceWled(const QJsonObject &deviceConfig)
9898
,_isSwitchOffOtherSegments(DEFAULT_IS_SWITCH_OFF_OTHER_SEGMENTS)
9999
,_isStreamToSegment(false)
100100
{
101-
TRACK_SCOPE_SUBCOMPONENT;;
101+
TRACK_SCOPE_SUBCOMPONENT;
102102
#ifdef ENABLE_MDNS
103103
QMetaObject::invokeMethod(MdnsBrowser::getInstance().data(), "browseForServiceType",
104104
Qt::QueuedConnection, Q_ARG(QByteArray, MdnsServiceRegister::getServiceType(_activeDeviceType)));

libsrc/utils/Logger.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,15 @@ void Logger::Message(LogLevel level, const char* sourceFile, const char* func, u
191191
{
192192
auto const globalLevel = static_cast<LogLevel>(int(GLOBAL_MIN_LOG_LEVEL));
193193

194-
if ((globalLevel == Logger::LogLevel::Unset && level < static_cast<LogLevel>(int(_minLevel))) // no global level, use level from logger
195-
|| (globalLevel > LogLevel::Unset && level < globalLevel)) // global level set, use global level
196-
{
197-
return;
198-
}
194+
// Determine the effective minimum level: prefer global if set, otherwise this logger's level
195+
const auto effectiveMinLevel = (globalLevel == Logger::LogLevel::Unset)
196+
? getMinLevel()
197+
: globalLevel;
198+
199+
if (level < effectiveMinLevel)
200+
{
201+
return;
202+
}
199203

200204
const size_t max_msg_length = 1024;
201205
char msg[max_msg_length];

0 commit comments

Comments
 (0)