From 440a94afbaca827a070119e0263770842125f036 Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 06:54:48 +0200 Subject: [PATCH 1/7] feat: introduce clang-tidy --- .clang-tidy | 52 ++++++++++ .github/workflows/build.yml | 7 +- CMakeLists.txt | 7 ++ src/receiver/decoder.cpp | 106 +++++++++++--------- src/receiver/decoder.h | 22 ++--- src/receiver/video_player.cpp | 125 ++++++++++++++---------- src/sender/encoder.cpp | 40 ++++---- src/sender/encoder.hpp | 29 +++--- src/sender/tcp_socket.cpp | 34 ++++--- src/sender/tcp_socket.hpp | 2 +- src/sender/usb_camera_frame_grabber.cpp | 11 ++- src/sender/usb_camera_frame_grabber.hpp | 2 +- src/sender/video_streamer_tcp.cpp | 109 +++++++++++---------- 13 files changed, 332 insertions(+), 214 deletions(-) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..fa19a68 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,52 @@ +FormatStyle: file + +Checks: ' +-*, +bugprone-*, +cppcoreguidelines-*, +google-*, +llvm-*, +misc-*, +modernize-*, +readability-*, +-llvmlibc-*, +-misc-include-cleaner, +-modernize-use-trailing-return-type, +-readability-implicit-bool-conversion, +-cppcoreguidelines-no-malloc, +-cppcoreguidelines-owning-memory, +-cppcoreguidelines-pro-bounds-pointer-arithmetic, +-cppcoreguidelines-pro-bounds-array-to-pointer-decay, +-cppcoreguidelines-pro-type-vararg, +-cppcoreguidelines-pro-type-reinterpret-cast, +-cppcoreguidelines-avoid-non-const-global-variables, +-readability-function-cognitive-complexity, +-readability-identifier-length, +-readability-magic-numbers, +-cppcoreguidelines-avoid-magic-numbers, +-modernize-avoid-c-arrays, +-cppcoreguidelines-avoid-c-arrays, +-bugprone-easily-swappable-parameters, +' + +WarningsAsErrors: '*' + +CheckOptions: + - key: readability-identifier-naming.NamespaceCase + value: lower_case + - key: readability-identifier-naming.ClassCase + value: CamelCase + - key: readability-identifier-naming.StructCase + value: CamelCase + - key: readability-identifier-naming.UnionCase + value: CamelCase + - key: readability-identifier-naming.EnumCase + value: CamelCase + - key: readability-identifier-naming.FunctionCase + value: camelBack + - key: readability-identifier-naming.MethodCase + value: camelBack + - key: readability-identifier-naming.VariableCase + value: camelBack + - key: readability-identifier-naming.ParameterCase + value: camelBack diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7e90b97..58475fc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,7 +22,12 @@ jobs: with: packages: build-essential cmake ffmpeg libx264-dev x264 libopencv-dev libva-dev libsdl2-dev version: 1.0 # bump this if you change the package list to invalidate the cache + - name: cache development apt packages + uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: clang-tidy + version: 1.0 # bump this if you change the package list to invalidate the cache - name: configure - run: mkdir -p build && cd build && cmake .. + run: mkdir -p build && cd build && cmake .. -DENABLE_CLANG_TIDY=ON - name: build run: cd build && cmake --build . diff --git a/CMakeLists.txt b/CMakeLists.txt index d1c898b..7a1f611 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,4 +7,11 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -O3 -march=native") +option(ENABLE_CLANG_TIDY "Run clang-tidy during compilation" OFF) +if(ENABLE_CLANG_TIDY) + find_program(CLANG_TIDY_EXE NAMES clang-tidy REQUIRED) + set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXE}") + message(STATUS "clang-tidy enabled: ${CLANG_TIDY_EXE}") +endif() + add_subdirectory(src) diff --git a/src/receiver/decoder.cpp b/src/receiver/decoder.cpp index bd6465a..79f4811 100644 --- a/src/receiver/decoder.cpp +++ b/src/receiver/decoder.cpp @@ -1,5 +1,7 @@ #include "decoder.h" +#include +#include #include // Socket includes @@ -9,19 +11,15 @@ #include #include -// Serial port includes -#include //Error number definitions - #include #include //File control definitions #include //POSIX terminal control definitions -#include // #define ARDUINO_MSMT -#define MSGLENGTH 30 +constexpr int kMsgLength = 30; -void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) +void Decoder(const char* videoAddress, uint8_t** argbRaw, bool* /*newImg*/) { //--------------------------------------------------------------------------------------------------- //---------------------------- Variable initializations @@ -29,12 +27,15 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) //--------------------------------------------------------------------------------------------------- // Timekeeping - std::chrono::time_point overall_start, cc_end, timekeeper, decoding_start, - decoding_end; + std::chrono::time_point overallStart; + std::chrono::time_point ccEnd; + std::chrono::time_point timekeeper; + std::chrono::time_point decodingStart; + std::chrono::time_point decodingEnd; // Color conversion - SwsContext* m_pSwsCtxYuv2Bgra; - int* argb_stride; + SwsContext* swsCtxYuv2Bgra = nullptr; + int* argbStride = nullptr; int framecounter = 0; #ifdef ARTIFICIAL_DELAY std::list pktlist; @@ -46,7 +47,7 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) #ifdef ARDUINO_MSMT // Arduino setup const char* ard_port = "/dev/ttyACM0"; - char buf[MSGLENGTH] = {0}; + char buf[kMsgLength] = {0}; int ard; ard = open(ard_port, O_RDWR | O_NOCTTY); @@ -118,9 +119,9 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) //---------------------------------------------- //--------------------------------------------------------------------------------------------------- // Simple startup without retrieving any stream info - AVCodecContext* cctx; - const AVCodec* codec; - AVFrame* frame; + AVCodecContext* cctx = nullptr; + const AVCodec* codec = nullptr; + AVFrame* frame = nullptr; #if LIBAVFORMAT_VERSION_INT < AV_VERSION_INT(58, 9, 100) av_register_all(); @@ -133,7 +134,7 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) cctx->height = height; cctx->pix_fmt = AV_PIX_FMT_YUV420P; - if (avcodec_open2(cctx, codec, NULL) < 0) + if (avcodec_open2(cctx, codec, nullptr) < 0) { std::cout << "Could not open decoder!\n"; readyToQuit = true; @@ -142,16 +143,16 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) frame = av_frame_alloc(); // Complex startup with retrieving stream info - int frameFinished; - AVFormatContext* fctx = NULL; + int frameFinished = 0; + AVFormatContext* fctx = nullptr; fctx = avformat_alloc_context(); /* open input file, and allocate format context */ std::cout << "Waiting for input..." << std::endl; - if (avformat_open_input(&fctx, video_address, NULL, NULL) < 0) + if (avformat_open_input(&fctx, videoAddress, nullptr, nullptr) < 0) { - fprintf(stderr, "Could not open source file %s\n", video_address); + fprintf(stderr, "Could not open source file %s\n", videoAddress); exit(1); } @@ -159,19 +160,19 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) //---------------------------- CSP conversion initialization //------------------------------------- //--------------------------------------------------------------------------------------------------- - m_pSwsCtxYuv2Bgra = sws_getContext( - width, height, AV_PIX_FMT_YUV420P, targetWidth, targetHeight, AV_PIX_FMT_RGBA, SWS_BILINEAR, NULL, NULL, NULL); + swsCtxYuv2Bgra = sws_getContext( + width, height, AV_PIX_FMT_YUV420P, targetWidth, targetHeight, AV_PIX_FMT_RGBA, SWS_BILINEAR, nullptr, nullptr, nullptr); - uint8_t* argb_data = (uint8_t*)malloc(targetWidth * targetHeight * 4 * sizeof(uint8_t)); - argb_raw[0] = argb_data; - argb_raw[1] = argb_data + targetWidth * targetHeight; - argb_raw[2] = argb_data + targetWidth * targetHeight * 2; - argb_raw[3] = argb_data + targetWidth * targetHeight * 3; - argb_stride = (int*)malloc(sizeof(int) * 1); - argb_stride[0] = 4 * targetWidth; + auto* argbData = static_cast(malloc(static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t))); + argbRaw[0] = argbData; + argbRaw[1] = argbData + static_cast(targetWidth) * targetHeight; + argbRaw[2] = argbData + static_cast(targetWidth) * targetHeight * 2; + argbRaw[3] = argbData + static_cast(targetWidth) * targetHeight * 3; + argbStride = static_cast(malloc(sizeof(int) * 1)); + argbStride[0] = 4 * targetWidth; timekeeper = std::chrono::high_resolution_clock::now(); - overall_start = std::chrono::high_resolution_clock::now(); + overallStart = std::chrono::high_resolution_clock::now(); //--------------------------------------------------------------------------------------------------- //----------------------------------------- Main Loop @@ -181,14 +182,15 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) while (!readyToQuit) { AVPacket* pkt = av_packet_alloc(); - int ret = av_read_frame(fctx, pkt); + const int ret = av_read_frame(fctx, pkt); // Packets with a size equal to 100 // bytes are dummy packets. // These packets just push the actual // packets through av_read_frame. We // don't want to // process them. - if ((ret >= 0) && (pkt->size > 100)) + constexpr int kMinPacketSize = 100; + if ((ret >= 0) && (pkt->size > kMinPacketSize)) { #ifdef ARTIFICIAL_DELAY @@ -203,23 +205,27 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) else { while (pktlist.size() >= listlength) + { pktlist.pop_front(); + } continue; } } #endif framecounter++; - decoding_start = std::chrono::high_resolution_clock::now(); + decodingStart = std::chrono::high_resolution_clock::now(); decode(cctx, frame, &frameFinished, pkt); - decoding_end = std::chrono::high_resolution_clock::now(); - sws_scale(m_pSwsCtxYuv2Bgra, frame->data, frame->linesize, 0, cctx->height, argb_raw, argb_stride); - cc_end = std::chrono::high_resolution_clock::now(); + decodingEnd = std::chrono::high_resolution_clock::now(); + sws_scale(swsCtxYuv2Bgra, frame->data, frame->linesize, 0, cctx->height, argbRaw, argbStride); + ccEnd = std::chrono::high_resolution_clock::now(); newImage = true; #ifdef ARTIFICIAL_DELAY - if (pktlist.size() > 0) + if (!pktlist.empty()) + { pktlist.pop_front(); + } #endif #ifdef ARDUINO_MSMT @@ -242,15 +248,17 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) std::cout << strbuf; } #else - if (framecounter % (int)(60 / 5) == 0) + constexpr int kFps = 60; + constexpr int kPrintInterval = 5; + if (framecounter % (kFps / kPrintInterval) == 0) { std::cout << "\r" << "rb=" << pkt->size << "B, " << "t_dec=" - << std::chrono::duration_cast(decoding_end - decoding_start).count() + << std::chrono::duration_cast(decodingEnd - decodingStart).count() << "us, t_cc=" - << std::chrono::duration_cast(cc_end - decoding_end).count() + << std::chrono::duration_cast(ccEnd - decodingEnd).count() << std::flush; } #endif @@ -268,18 +276,18 @@ void Decoder(const char* video_address, uint8_t** argb_raw, bool* newImg) close(ard); #endif - sws_freeContext(m_pSwsCtxYuv2Bgra); - free(argb_raw); - free(argb_stride); + sws_freeContext(swsCtxYuv2Bgra); + free(reinterpret_cast(argbRaw)); + free(argbStride); std::cout << "Successfully terminated decoder thread" << std::endl; } -int decode(AVCodecContext* avctx, AVFrame* frame, int* got_frame, AVPacket* pkt) +int decode(AVCodecContext* avctx, AVFrame* frame, int* gotFrame, AVPacket* pkt) { - int ret; + int ret = 0; - *got_frame = 0; + *gotFrame = 0; if (pkt) { @@ -289,14 +297,20 @@ int decode(AVCodecContext* avctx, AVFrame* frame, int* got_frame, AVPacket* pkt) // decoded frames with // avcodec_receive_frame() until done. if (ret < 0) + { return ret == AVERROR_EOF ? 0 : ret; + } } ret = avcodec_receive_frame(avctx, frame); if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF) + { return ret; + } if (ret >= 0) - *got_frame = 1; + { + *gotFrame = 1; + } return 0; } diff --git a/src/receiver/decoder.h b/src/receiver/decoder.h index 1f6d307..679248c 100644 --- a/src/receiver/decoder.h +++ b/src/receiver/decoder.h @@ -2,8 +2,8 @@ #define DECODER_H_ // System includes -#include -#include +#include +#include #include #include @@ -24,23 +24,23 @@ extern "C" { extern unsigned int listlength; #endif -extern const char* video_url; -extern const int video_port; -extern const char* ptu_url; -extern const int to_ptu_port; -extern const int from_ptu_port; +extern const char* videoUrl; +extern const int videoPort; +extern const char* ptuUrl; +extern const int toPtuPort; +extern const int fromPtuPort; -extern uint8_t** argb_src; +extern uint8_t** argbSrc; extern bool newImage; extern bool readyToQuit; extern int height, width; extern const int targetWidth, targetHeight, stereoheight; extern char* messageFromPTU; -void Decoder(const char* video_address, - uint8_t** argb_raw, +void Decoder(const char* videoAddress, + uint8_t** argbRaw, bool* newImg); // This is the decoder thread running in parallel to the main thread -int decode(AVCodecContext* avctx, AVFrame* frame, int* got_frame, AVPacket* pkt); +int decode(AVCodecContext* avctx, AVFrame* frame, int* gotFrame, AVPacket* pkt); #endif /* DECODER_H_ */ diff --git a/src/receiver/video_player.cpp b/src/receiver/video_player.cpp index b4213e9..d4e6264 100644 --- a/src/receiver/video_player.cpp +++ b/src/receiver/video_player.cpp @@ -1,3 +1,7 @@ +#include +#include +#include +#include #include #include @@ -6,11 +10,11 @@ #include "decoder.h" -const char* video_url = "tcp://127.0.0.1:5001"; -// const char * video_url = "tcp://10.152.4.207:5000"; -// const char * video_url = "tcp://10.152.4.195:5000"; +const char* videoUrl = "tcp://127.0.0.1:5001"; +// const char * videoUrl = "tcp://10.152.4.207:5000"; +// const char * videoUrl = "tcp://10.152.4.195:5000"; int port = 5002; -const char* target_ip = "127.0.0.1"; +const char* targetIp = "127.0.0.1"; bool video = true; bool fullscreen = false; int vsync = 1; @@ -18,70 +22,77 @@ const int targetWidth = 1280, targetHeight = 720; int height = 720, width = 1280; // will be overwritten by the video dimensions in the decoder; definition here // necessary if no video is used -bool readyToQuit = false; // Quits all threads -uint8_t** argb_src = (uint8_t**)malloc(sizeof(uint8_t*) * 4); // Pointer to the decoded image +bool readyToQuit = false; // Quits all threads +uint8_t** argbSrc = static_cast(malloc(sizeof(uint8_t*) * 4)); // Pointer to the decoded image bool newImage = false; // Set to true by the decoder when it decodes a frame. Set to false after oculus as copied a decoded frame #ifdef ARTIFICIAL_DELAY unsigned int listlength = 0; #endif -using namespace std; int main(int argc, char* argv[]) { if (argc == 2) { - video_url = argv[1]; + videoUrl = argv[1]; } else { - cout << "Decoding video from default url:" << video_url << ". For other sources, use e.g. '" << argv[0] + std::cout << "Decoding video from default url:" << videoUrl << ". For other sources, use e.g. '" << argv[0] << " tcp://10.152.4.207:5000'\n"; } // SDL Inits if (SDL_Init(SDL_INIT_VIDEO) < 0) - cout << "Failed to initialize SDL! SDL Error :" << SDL_GetError() << endl; + { + std::cout << "Failed to initialize SDL! SDL Error :" << SDL_GetError() << std::endl; + } - int display = 0; + const int display = 0; // if (SDL_GetNumVideoDisplays() > 1) { // cout << "Choose display for video output.\n\n 0 = default desktop,\n 1 = secondary screen,\n etc." << // endl; // cin >> display; // } - SDL_Event* ev = new SDL_Event(); - - uint8_t* pixel_data = (uint8_t*)malloc(targetWidth * targetHeight * 4 * sizeof(uint8_t)); - uint8_t** pixels; - int* argb_stride; - pixels = (uint8_t**)malloc(sizeof(uint8_t*) * 4); - pixels[0] = pixel_data; - pixels[1] = pixel_data + targetWidth * targetHeight; - pixels[2] = pixel_data + targetWidth * targetHeight * 2; - pixels[3] = pixel_data + targetWidth * targetHeight * 3; - argb_stride = (int*)malloc(sizeof(int) * 1); - argb_stride[0] = 4 * targetWidth; - int test[1]; + auto* event = new SDL_Event(); + + auto* pixelData = static_cast(malloc(static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t))); + uint8_t** pixels = nullptr; + int* argbStride = nullptr; + pixels = static_cast(malloc(sizeof(uint8_t*) * 4)); + pixels[0] = pixelData; + pixels[1] = pixelData + static_cast(targetWidth) * targetHeight; + pixels[2] = pixelData + static_cast(targetWidth) * targetHeight * 2; + pixels[3] = pixelData + static_cast(targetWidth) * targetHeight * 3; + argbStride = static_cast(malloc(sizeof(int) * 1)); + argbStride[0] = 4 * targetWidth; + std::array test = {}; test[0] = targetWidth * 4; - thread dec; + std::thread dec; if (video) - dec = thread(Decoder, video_url, argb_src, &newImage); + { + dec = std::thread(Decoder, videoUrl, argbSrc, &newImage); + } //------------------ UDP port setup --------------- - struct sockaddr_in remaddr; - int fd, slen = sizeof(remaddr); + struct sockaddr_in remaddr {}; + int sockFd = 0; + const int slen = sizeof(remaddr); //-------------------------- UDP Port setup ----------------------- - if ((fd = socket(AF_INET, SOCK_DGRAM, 0)) == -1) - cout << "Could not create socket!" << endl; + sockFd = socket(AF_INET, SOCK_DGRAM, 0); + if (sockFd == -1) + { + std::cout << "Could not create socket!" << std::endl; + } - memset((char*)&remaddr, 0, sizeof(remaddr)); + memset(reinterpret_cast(&remaddr), 0, sizeof(remaddr)); remaddr.sin_family = AF_INET; remaddr.sin_port = htons(port); - if (inet_aton(target_ip, &remaddr.sin_addr) == 0) + if (inet_aton(targetIp, &remaddr.sin_addr) == 0) { fprintf(stderr, "inet_aton() failed\n"); exit(1); @@ -95,7 +106,7 @@ int main(int argc, char* argv[]) { std::this_thread::sleep_for(std::chrono::milliseconds(20)); } - cout << "Got first image, now displaying" << endl; + std::cout << "Got first image, now displaying" << std::endl; } SDL_Window* win = SDL_CreateWindow("Videoplayer", @@ -105,16 +116,22 @@ int main(int argc, char* argv[]) targetHeight, SDL_WINDOW_OPENGL); if (!win) - cout << "Failed to create Window! SDL Error :" << SDL_GetError() << endl; + { + std::cout << "Failed to create Window! SDL Error :" << SDL_GetError() << std::endl; + } SDL_Renderer* ren = SDL_CreateRenderer(win, -1, SDL_RENDERER_ACCELERATED); if (!ren) - cout << "Failed to create Renderer! SDL Error :" << SDL_GetError() << endl; + { + std::cout << "Failed to create Renderer! SDL Error :" << SDL_GetError() << std::endl; + } SDL_Texture* tex = SDL_CreateTexture(ren, SDL_PIXELFORMAT_ABGR8888, SDL_TEXTUREACCESS_STREAMING, targetWidth, targetHeight); if (!tex) - cout << "Failed to create Texture! SDL Error :" << SDL_GetError() << endl; + { + std::cout << "Failed to create Texture! SDL Error :" << SDL_GetError() << std::endl; + } if (fullscreen) { @@ -128,16 +145,18 @@ int main(int argc, char* argv[]) while (!readyToQuit) { // Polling for user signal to end - while (SDL_PollEvent(ev)) + while (SDL_PollEvent(event)) { - if (ev->type == SDL_KEYDOWN) + if (event->type == SDL_KEYDOWN) { - if ((ev->key.keysym.sym == SDLK_q) || (ev->key.keysym.sym == SDLK_RETURN) || - (ev->key.keysym.sym == SDLK_SPACE) || (ev->key.keysym.sym == SDLK_ESCAPE)) + if ((event->key.keysym.sym == SDLK_q) || (event->key.keysym.sym == SDLK_RETURN) || + (event->key.keysym.sym == SDLK_SPACE) || (event->key.keysym.sym == SDLK_ESCAPE)) + { readyToQuit = true; + } - const char* msg; - switch (ev->key.keysym.sym) + const char* msg = nullptr; + switch (event->key.keysym.sym) { case SDLK_m: msg = "m"; @@ -159,7 +178,9 @@ int main(int argc, char* argv[]) break; // Decrease box size case SDLK_o: if (listlength > 0) - listlength = max((unsigned int)0, listlength - 1); + { + listlength = std::max(0U, listlength - 1); + } std::cout << "\n" << listlength << "\n"; msg = "o"; break; // Decrease delay @@ -171,13 +192,13 @@ int main(int argc, char* argv[]) default: break; } - sendto(fd, msg, sizeof(char), 0, (struct sockaddr*)&remaddr, slen); + sendto(sockFd, msg, sizeof(char), 0, reinterpret_cast(&remaddr), slen); } else { - if ((ev->type == SDL_MOUSEBUTTONDOWN)) + if ((event->type == SDL_MOUSEBUTTONDOWN)) { // Shoot! - sendto(fd, "t", sizeof(char), 0, (struct sockaddr*)&remaddr, slen); + sendto(sockFd, "t", sizeof(char), 0, reinterpret_cast(&remaddr), slen); } } } @@ -185,17 +206,19 @@ int main(int argc, char* argv[]) // Updating image if new one has been decoded if (newImage) { - SDL_LockTexture(tex, NULL, reinterpret_cast(pixels), test); + SDL_LockTexture(tex, nullptr, reinterpret_cast(pixels), test.data()); // Copy decoded image into retrieved pointer from locking // m_start_time = std::chrono::high_resolution_clock::now(); if (video) - memcpy(*pixels, *argb_src, targetWidth * targetHeight * 4 * sizeof(uint8_t)); + { + memcpy(*pixels, *argbSrc, static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t)); + } // m_end_time = std::chrono::high_resolution_clock::now(); SDL_UnlockTexture(tex); SDL_RenderClear(ren); - SDL_RenderCopy(ren, tex, NULL, NULL); + SDL_RenderCopy(ren, tex, nullptr, nullptr); SDL_RenderPresent(ren); // Reset the newImage flag @@ -208,7 +231,9 @@ int main(int argc, char* argv[]) readyToQuit = true; if (video) + { dec.join(); + } // Shutting down SDL SDL_DestroyTexture(tex); @@ -216,7 +241,7 @@ int main(int argc, char* argv[]) SDL_DestroyWindow(win); SDL_Quit(); - cout << "Quit video player" << endl; + std::cout << "Quit video player" << std::endl; return 0; } diff --git a/src/sender/encoder.cpp b/src/sender/encoder.cpp index ea4c325..6c2f5d8 100644 --- a/src/sender/encoder.cpp +++ b/src/sender/encoder.cpp @@ -9,15 +9,14 @@ Encoder::~Encoder() // Setup of the encoder instance Encoder::Encoder(int inW, int inH, int outW, int outH, float fps) - : in_xres(inW), in_yres(inH), out_xres(outW), out_yres(outH) + : inXres(inW), inYres(inH), outXres(outW), outYres(outH) { - framecounter = 0; x264_param_default_preset(&prms, "ultrafast", "zerolatency,fastdecode"); x264_param_apply_profile(&prms, "baseline"); - prms.i_width = out_xres; - prms.i_height = out_yres; - prms.i_fps_num = fps; + prms.i_width = outXres; + prms.i_height = outYres; + prms.i_fps_num = static_cast(fps); prms.i_fps_den = 1; prms.rc.i_qp_constant = 20; @@ -30,19 +29,19 @@ Encoder::Encoder(int inW, int inH, int outW, int outH, float fps) x264_encoder_headers(enc, &nals, &nheader); // Initialize X264 Picture - x264_picture_alloc(&pic_in, X264_CSP_I420, out_xres, out_yres); + x264_picture_alloc(&pic_in, X264_CSP_I420, outXres, outYres); // Color conversion setup - sws = sws_getContext(in_xres, - in_yres, - cam_pixel_fmt, // AV_PIX_FMT_BAYER_GBRG8, AV_PIX_FMT_RGB24 - out_xres, - out_yres, + sws = sws_getContext(inXres, + inYres, + camPixelFmt, // AV_PIX_FMT_BAYER_GBRG8, AV_PIX_FMT_RGB24 + outXres, + outYres, AV_PIX_FMT_YUV420P, SWS_FAST_BILINEAR, - NULL, - NULL, - NULL); + nullptr, + nullptr, + nullptr); if (!sws) { @@ -55,19 +54,18 @@ int Encoder::encode(unsigned char* img, bool* imgReady) { // Put raw image data to AV picture - int bytes_filled = av_image_fill_arrays(pic_raw.data, pic_raw.linesize, img, cam_pixel_fmt, in_xres, in_yres, 1); - if (!bytes_filled) + const int bytesFilled = av_image_fill_arrays(picRaw.data, picRaw.linesize, img, camPixelFmt, inXres, inYres, 1); + if (!bytesFilled) { std::cout << "Cannot fill the raw input buffer" << std::endl; return -1; } // convert to I420 for x264 - int h = sws_scale(sws, pic_raw.data, pic_raw.linesize, 0, in_yres, pic_in.img.plane, pic_in.img.i_stride); - if (h != out_yres) + const int h = sws_scale(sws, picRaw.data, picRaw.linesize, 0, inYres, pic_in.img.plane, pic_in.img.i_stride); + if (h != outYres) { std::cout << "scale failed: %d" << std::endl; - ; return -1; } @@ -77,7 +75,7 @@ int Encoder::encode(unsigned char* img, bool* imgReady) // Encode pic_in.i_pts = framecounter++; - int frame_size = x264_encoder_encode(enc, &nals, &num_nals, &pic_in, &pic_out); + const int frameSize = x264_encoder_encode(enc, &nals, &numNals, &pic_in, &pic_out); - return frame_size; + return frameSize; } diff --git a/src/sender/encoder.hpp b/src/sender/encoder.hpp index 33307f6..c3d4574 100644 --- a/src/sender/encoder.hpp +++ b/src/sender/encoder.hpp @@ -1,8 +1,7 @@ - #ifndef ENCODER_HPP_ #define ENCODER_HPP_ -#include +#include extern "C" { #include @@ -16,20 +15,24 @@ extern "C" { class Encoder { private: - int in_xres, in_yres, out_xres, out_yres; - int framecounter; - int nheader; - x264_t* enc; - x264_param_t prms; - x264_picture_t pic_in, pic_out; + int inXres = 0; + int inYres = 0; + int outXres = 0; + int outYres = 0; + int framecounter = 0; + int nheader = 0; + x264_t* enc = nullptr; + x264_param_t prms {}; + x264_picture_t pic_in {}; + x264_picture_t pic_out {}; - struct SwsContext* sws; - AVFrame pic_raw; /* used for our "raw" input container */ - AVPixelFormat cam_pixel_fmt = AV_PIX_FMT_BGR24; + struct SwsContext* sws = nullptr; + AVFrame picRaw {}; + AVPixelFormat camPixelFmt = AV_PIX_FMT_BGR24; public: - x264_nal_t* nals; - int num_nals; + x264_nal_t* nals = nullptr; + int numNals = 0; virtual ~Encoder(); Encoder() {}; Encoder(int, int, int, int, float); diff --git a/src/sender/tcp_socket.cpp b/src/sender/tcp_socket.cpp index ec57207..121538d 100644 --- a/src/sender/tcp_socket.cpp +++ b/src/sender/tcp_socket.cpp @@ -1,8 +1,8 @@ #include "tcp_socket.hpp" -#include -#include -#include +#include +#include +#include #include @@ -13,8 +13,8 @@ #include TcpSocket::TcpSocket(int port) + : portID(port), sockID(0) { - portID = port; } TcpSocket::~TcpSocket() @@ -26,32 +26,34 @@ TcpSocket::~TcpSocket() int TcpSocket::listenForLocalConnection() { - int tempsockID = 0; - struct sockaddr_in remaddr, cli_addr; - socklen_t clilen; + int tempSockId = 0; + struct sockaddr_in remaddr {}; + struct sockaddr_in cliAddr {}; + socklen_t clilen = 0; - if ((tempsockID = socket(AF_INET, SOCK_STREAM, 0)) < 0) + tempSockId = socket(AF_INET, SOCK_STREAM, 0); + if (tempSockId < 0) { std::cout << "Could not create socket!\n"; // SOCK_STREAM invokes TCP, SOCK_DGRAM would invoke UDP return 0; } - memset((char*)&remaddr, 0, sizeof(remaddr)); + memset(reinterpret_cast(&remaddr), 0, sizeof(remaddr)); remaddr.sin_family = AF_INET; remaddr.sin_addr.s_addr = INADDR_ANY; // For TCP, we are listening on our own socket remaddr.sin_port = htons(portID); // For TCP - if (bind(tempsockID, (struct sockaddr*)&remaddr, sizeof(remaddr)) < 0) + if (bind(tempSockId, reinterpret_cast(&remaddr), sizeof(remaddr)) < 0) { std::cout << "Error on socket binding!\n"; return 0; } - listen(tempsockID, 5); - clilen = sizeof(cli_addr); + listen(tempSockId, 5); + clilen = sizeof(cliAddr); std::cout << "Waiting for TCP connection...\n"; - sockID = accept(tempsockID, (struct sockaddr*)&cli_addr, &clilen); + sockID = accept(tempSockId, reinterpret_cast(&cliAddr), &clilen); if (sockID < 0) { std::cout << "Error on accepting TCP connection!\n"; @@ -59,13 +61,13 @@ int TcpSocket::listenForLocalConnection() } // Temporary socket not needed anymore, closing - close(tempsockID); + close(tempSockId); std::cout << "Established TCP connection!\n"; return 1; } -int TcpSocket::send(unsigned char* addr, int len) +int TcpSocket::send(unsigned char* addr, int len) const { - return write(sockID, addr, len); + return static_cast(write(sockID, addr, len)); } diff --git a/src/sender/tcp_socket.hpp b/src/sender/tcp_socket.hpp index af59bcb..dc57dc0 100644 --- a/src/sender/tcp_socket.hpp +++ b/src/sender/tcp_socket.hpp @@ -15,7 +15,7 @@ class TcpSocket // Listens for a TCP connection on a local port returns 1 on successful connection, 0 on error int listenForLocalConnection(); - int send(unsigned char*, int); + int send(unsigned char*, int) const; }; #endif diff --git a/src/sender/usb_camera_frame_grabber.cpp b/src/sender/usb_camera_frame_grabber.cpp index b6d15a9..a4d3626 100644 --- a/src/sender/usb_camera_frame_grabber.cpp +++ b/src/sender/usb_camera_frame_grabber.cpp @@ -1,6 +1,7 @@ #include "usb_camera_frame_grabber.hpp" -#include +#include +#include #include @@ -8,7 +9,7 @@ #include #include -void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgReady, bool* prog_end) +void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgReady, bool* progEnd) { // Retrieving a handle to the camera device @@ -17,7 +18,7 @@ void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgR if (!cap.isOpened()) { std::cout << "Could not open camera!\n"; - *prog_end = true; + *progEnd = true; } // Set camera parameters @@ -33,14 +34,14 @@ void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgR cv::Mat frame; // Main loop for acquiring images from the camera - while (!(*prog_end)) + while (!(*progEnd)) { // Get image cap >> frame; // get a new frame from camera // Copy to allocated memory from main - memcpy(img, frame.data, (params->height) * (params->width) * 3); + memcpy(img, frame.data, static_cast(params->height) * params->width * 3); // Set ReadyRead: encoder can now process image *imgReady = true; diff --git a/src/sender/usb_camera_frame_grabber.hpp b/src/sender/usb_camera_frame_grabber.hpp index bd14064..c980ae9 100644 --- a/src/sender/usb_camera_frame_grabber.hpp +++ b/src/sender/usb_camera_frame_grabber.hpp @@ -12,4 +12,4 @@ struct CameraParameters float color_coeffs[3]; }; -void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* readyRead, bool* prog_end); +void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgReady, bool* progEnd); diff --git a/src/sender/video_streamer_tcp.cpp b/src/sender/video_streamer_tcp.cpp index b7f9ff8..009014c 100644 --- a/src/sender/video_streamer_tcp.cpp +++ b/src/sender/video_streamer_tcp.cpp @@ -4,9 +4,9 @@ #include "usb_camera_frame_grabber.hpp" #endif -#include -#include -#include +#include +#include +#include #include #include @@ -21,27 +21,32 @@ // Parses the command line arguments and sets up the camera and encoder objects // Returns 1 on success, 0 on failure -int argumentParser(int, char**, TcpSocket*, CameraParameters*, Encoder*); +int argumentParser(int argc, char** argv, TcpSocket* sock, CameraParameters* camParams, Encoder* enc); int main(int argc, char* argv[]) { // Variable initializations - bool prog_end = false, // Shared between threads to signal program shutdown - imgReady = false; // Synchronizes cameraFrameGrabber and encoder + bool progEnd = false; // Shared between threads to signal program shutdown + bool imgReady = false; // Synchronizes cameraFrameGrabber and encoder int framecounter = 0; - std::chrono::time_point enc_start, enc_end, timekeeper, overall_start; + std::chrono::time_point encStart; + std::chrono::time_point encEnd; + std::chrono::time_point timekeeper; + std::chrono::time_point overallStart; // Main objects: camera parameters, encoder and tcp Socket - CameraParameters cam_params; + CameraParameters camParams {}; Encoder enc; TcpSocket sock; // Parse command line arguments - if (!argumentParser(argc, argv, &sock, &cam_params, &enc)) + if (!argumentParser(argc, argv, &sock, &camParams, &enc)) + { return -1; + } // Allocate memory for raw image - unsigned char* img = (unsigned char*)malloc(cam_params.width * cam_params.height * 3); + auto* img = static_cast(malloc(static_cast(camParams.width) * camParams.height * 3)); // Listen for connection request if (!sock.listenForLocalConnection()) @@ -50,55 +55,61 @@ int main(int argc, char* argv[]) } // Start camera frame grabber thread - std::thread camFrameGrabber(cameraFrameGrabber, &cam_params, img, &imgReady, &prog_end); + std::thread camFrameGrabber(cameraFrameGrabber, &camParams, img, &imgReady, &progEnd); // Main Loop timekeeper = std::chrono::high_resolution_clock::now(); - while (!prog_end) + while (!progEnd) { // First, wait for imgReady. Gets true once the cameraFrameGrabber has put a new image into the shared memory // img if (framecounter == 1) - overall_start = std::chrono::high_resolution_clock::now(); // Start measuring, when we have the first image - while ((!imgReady) && !prog_end) + { + overallStart = std::chrono::high_resolution_clock::now(); // Start measuring, when we have the first image + } + while ((!imgReady) && !progEnd) + { std::this_thread::sleep_for(std::chrono::microseconds(10)); - if (prog_end) + } + if (progEnd) + { break; + } // Encode - enc_start = std::chrono::high_resolution_clock::now(); - int frame_size = enc.encode(img, &imgReady); - enc_end = std::chrono::high_resolution_clock::now(); + encStart = std::chrono::high_resolution_clock::now(); + const int frameSize = enc.encode(img, &imgReady); + encEnd = std::chrono::high_resolution_clock::now(); // Send using tcpSocket - int sentbytes_frame = 0; - for (int i = 0; i < enc.num_nals; i++) + int sentbytesFrame = 0; + for (int i = 0; i < enc.numNals; i++) { - int sentbytes_nal = sock.send(enc.nals[i].p_payload, enc.nals[i].i_payload); + const int sentbytesNal = sock.send(enc.nals[i].p_payload, enc.nals[i].i_payload); - if (sentbytes_nal < 0) + if (sentbytesNal < 0) { // Video receiver shut down connection, quit this program - prog_end = true; + progEnd = true; std::this_thread::sleep_for(std::chrono::milliseconds(100)); std::cout << "\n\nClient closed TCP connection. Closing program.\n\n"; break; } - sentbytes_frame += sentbytes_nal; + sentbytesFrame += sentbytesNal; } timekeeper = std::chrono::high_resolution_clock::now(); // Put info to terminal - if ((framecounter % ((int)(cam_params.fps) / 5)) == 0) + if ((framecounter % (static_cast(camParams.fps) / 5)) == 0) { std::cout << "\rt_enc=" - << std::chrono::duration_cast(enc_end - enc_start).count() - << "us, fs=" << frame_size << "b, sb=" << sentbytes_frame << "b, fps=" - << (double)(framecounter * 1000) / - (std::chrono::duration_cast(timekeeper - overall_start).count()) + << std::chrono::duration_cast(encEnd - encStart).count() + << "us, fs=" << frameSize << "b, sb=" << sentbytesFrame << "b, fps=" + << (static_cast(framecounter) * 1000) / + static_cast(std::chrono::duration_cast(timekeeper - overallStart).count()) << "Hz. " << std::flush; } @@ -109,28 +120,28 @@ int main(int argc, char* argv[]) return 0; } -int argumentParser(int argc, char* argv[], TcpSocket* sock, CameraParameters* cam_params, Encoder* enc) +int argumentParser(int argc, char* argv[], TcpSocket* sock, CameraParameters* camParams, Encoder* enc) { std::stringstream ss; cv::FileStorage conf; std::string temp; - char* default_path = (char*)"../src/config.yaml"; + const char* defaultPath = "../src/config.yaml"; ss << "Usage: '" << argv[0] << " [path/to/config.yaml]'\n" << "[path/to/config.yaml] path to config file, per default in src/config.yaml.\n" << "Example: '" << argv[0] << " ../src/config.yaml'. \n\n"; - std::string msg = ss.str(); + const std::string msg = ss.str(); switch (argc) { case 1: - std::cout << "Called without arguments. Defaulting to path " << default_path << " for config file.\n" + std::cout << "Called without arguments. Defaulting to path " << defaultPath << " for config file.\n" << "Use 'ffplay -probesize 32 -sync ext tcp://127.0.0.1:5001' to view locally (or this " "computer's IP address instead of 127.0.0.1 to view somewhere else).\n" << "Careful, ffplay introduces noticeable lag through buffering\n\n"; std::cout << msg << std::endl; - conf = cv::FileStorage(default_path, cv::FileStorage::READ); + conf = cv::FileStorage(defaultPath, cv::FileStorage::READ); break; case 2: conf = cv::FileStorage(argv[1], cv::FileStorage::READ); @@ -142,25 +153,25 @@ int argumentParser(int argc, char* argv[], TcpSocket* sock, CameraParameters* ca // Get values from config file *sock = TcpSocket(conf["port"]); - cam_params->eye = (char*)malloc(99); - - temp = (std::string)conf["camera.name"]; - temp.copy(cam_params->eye, 99); - cam_params->width = 2 * ((int)conf["camera.width"]) / 2; - cam_params->height = 4 * ((int)conf["camera.height"]) / 4; - cam_params->sensor_width = conf["camera.sensor_width"]; - cam_params->sensor_height = conf["camera.sensor_height"]; - cam_params->fps = conf["camera.fps"]; - cam_params->color_coeffs[0] = conf["camera.r_coeff"]; - cam_params->color_coeffs[1] = conf["camera.g_coeff"]; - cam_params->color_coeffs[2] = conf["camera.b_coeff"]; + camParams->eye = static_cast(malloc(99)); + + temp = static_cast(conf["camera.name"]); + temp.copy(camParams->eye, 99); + camParams->width = 2 * (static_cast(conf["camera.width"])) / 2; + camParams->height = 4 * (static_cast(conf["camera.height"])) / 4; + camParams->sensor_width = conf["camera.sensor_width"]; + camParams->sensor_height = conf["camera.sensor_height"]; + camParams->fps = conf["camera.fps"]; + camParams->color_coeffs[0] = conf["camera.r_coeff"]; + camParams->color_coeffs[1] = conf["camera.g_coeff"]; + camParams->color_coeffs[2] = conf["camera.b_coeff"]; *enc = Encoder(conf["camera.width"], conf["camera.height"], conf["video.width"], conf["video.height"], conf["fps"]); // Computing remaining values - cam_params->t_exp = (int)(1000000 / (cam_params->fps * 1.005)); - cam_params->xoff = 16 * ((cam_params->sensor_width - cam_params->width) / (16 * 2)); - cam_params->yoff = 16 * ((cam_params->sensor_height - cam_params->height) / (16 * 2)); + camParams->t_exp = static_cast(1000000 / (camParams->fps * 1.005)); + camParams->xoff = 16 * ((camParams->sensor_width - camParams->width) / (16 * 2)); + camParams->yoff = 16 * ((camParams->sensor_height - camParams->height) / (16 * 2)); return 1; } From 9ee7d78e65ef5b39969d234a84006ec4d3eb88cd Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 07:00:10 +0200 Subject: [PATCH 2/7] enable readability-implicit-bool-conversion --- .clang-tidy | 1 - src/receiver/decoder.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index fa19a68..a08335a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -12,7 +12,6 @@ readability-*, -llvmlibc-*, -misc-include-cleaner, -modernize-use-trailing-return-type, --readability-implicit-bool-conversion, -cppcoreguidelines-no-malloc, -cppcoreguidelines-owning-memory, -cppcoreguidelines-pro-bounds-pointer-arithmetic, diff --git a/src/receiver/decoder.cpp b/src/receiver/decoder.cpp index 79f4811..53c14b9 100644 --- a/src/receiver/decoder.cpp +++ b/src/receiver/decoder.cpp @@ -289,7 +289,7 @@ int decode(AVCodecContext* avctx, AVFrame* frame, int* gotFrame, AVPacket* pkt) *gotFrame = 0; - if (pkt) + if (pkt != nullptr) { ret = avcodec_send_packet(avctx, pkt); // In particular, we don't expect From 861307973902f57305fc92fe406df6d7743089da Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 10:23:13 +0200 Subject: [PATCH 3/7] Avoid magic numbers --- .clang-tidy | 2 -- src/receiver/video_player.cpp | 15 ++++----- src/sender/encoder.cpp | 14 ++++++--- src/sender/tcp_socket.cpp | 13 ++++---- src/sender/tcp_socket.hpp | 4 +-- src/sender/usb_camera_frame_grabber.cpp | 6 ++-- src/sender/video_streamer_tcp.cpp | 42 ++++++++++++++----------- 7 files changed, 54 insertions(+), 42 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index a08335a..e8938c7 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -21,8 +21,6 @@ readability-*, -cppcoreguidelines-avoid-non-const-global-variables, -readability-function-cognitive-complexity, -readability-identifier-length, --readability-magic-numbers, --cppcoreguidelines-avoid-magic-numbers, -modernize-avoid-c-arrays, -cppcoreguidelines-avoid-c-arrays, -bugprone-easily-swappable-parameters, diff --git a/src/receiver/video_player.cpp b/src/receiver/video_player.cpp index d4e6264..350d00a 100644 --- a/src/receiver/video_player.cpp +++ b/src/receiver/video_player.cpp @@ -13,13 +13,13 @@ const char* videoUrl = "tcp://127.0.0.1:5001"; // const char * videoUrl = "tcp://10.152.4.207:5000"; // const char * videoUrl = "tcp://10.152.4.195:5000"; -int port = 5002; +const int port = 5002; const char* targetIp = "127.0.0.1"; bool video = true; bool fullscreen = false; int vsync = 1; const int targetWidth = 1280, targetHeight = 720; -int height = 720, width = 1280; // will be overwritten by the video dimensions in the decoder; definition here +int height = targetHeight, width = targetWidth; // will be overwritten by the video dimensions in the decoder; definition here // necessary if no video is used bool readyToQuit = false; // Quits all threads @@ -104,7 +104,8 @@ int main(int argc, char* argv[]) // cout << "Waiting for first decoded image..." << endl; while (!newImage) { - std::this_thread::sleep_for(std::chrono::milliseconds(20)); + const auto sleepDurationMs = 20; + std::this_thread::sleep_for(std::chrono::milliseconds(sleepDurationMs)); } std::cout << "Got first image, now displaying" << std::endl; } @@ -115,20 +116,20 @@ int main(int argc, char* argv[]) targetWidth, targetHeight, SDL_WINDOW_OPENGL); - if (!win) + if (win == nullptr) { std::cout << "Failed to create Window! SDL Error :" << SDL_GetError() << std::endl; } SDL_Renderer* ren = SDL_CreateRenderer(win, -1, SDL_RENDERER_ACCELERATED); - if (!ren) + if (ren == nullptr) { std::cout << "Failed to create Renderer! SDL Error :" << SDL_GetError() << std::endl; } SDL_Texture* tex = SDL_CreateTexture(ren, SDL_PIXELFORMAT_ABGR8888, SDL_TEXTUREACCESS_STREAMING, targetWidth, targetHeight); - if (!tex) + if (tex == nullptr) { std::cout << "Failed to create Texture! SDL Error :" << SDL_GetError() << std::endl; } @@ -145,7 +146,7 @@ int main(int argc, char* argv[]) while (!readyToQuit) { // Polling for user signal to end - while (SDL_PollEvent(event)) + while (static_cast(SDL_PollEvent(event))) { if (event->type == SDL_KEYDOWN) { diff --git a/src/sender/encoder.cpp b/src/sender/encoder.cpp index 6c2f5d8..84be98f 100644 --- a/src/sender/encoder.cpp +++ b/src/sender/encoder.cpp @@ -12,17 +12,21 @@ Encoder::Encoder(int inW, int inH, int outW, int outH, float fps) : inXres(inW), inYres(inH), outXres(outW), outYres(outH) { + const auto quantizationParameter = 20; + const auto rateFactor = 20.0F; + const auto rateFactorMax = 25.0F; + x264_param_default_preset(&prms, "ultrafast", "zerolatency,fastdecode"); x264_param_apply_profile(&prms, "baseline"); prms.i_width = outXres; prms.i_height = outYres; prms.i_fps_num = static_cast(fps); prms.i_fps_den = 1; - prms.rc.i_qp_constant = 20; + prms.rc.i_qp_constant = quantizationParameter; prms.rc.i_rc_method = X264_RC_CRF; - prms.rc.f_rf_constant = 20; - prms.rc.f_rf_constant_max = 25; + prms.rc.f_rf_constant = rateFactor; + prms.rc.f_rf_constant_max = rateFactorMax; prms.i_csp = X264_CSP_I420; enc = x264_encoder_open(&prms); @@ -43,7 +47,7 @@ Encoder::Encoder(int inW, int inH, int outW, int outH, float fps) nullptr, nullptr); - if (!sws) + if (sws == nullptr) { std::cout << "Cannot create SWS context" << std::endl; } @@ -55,7 +59,7 @@ int Encoder::encode(unsigned char* img, bool* imgReady) // Put raw image data to AV picture const int bytesFilled = av_image_fill_arrays(picRaw.data, picRaw.linesize, img, camPixelFmt, inXres, inYres, 1); - if (!bytesFilled) + if (bytesFilled <= 0) { std::cout << "Cannot fill the raw input buffer" << std::endl; return -1; diff --git a/src/sender/tcp_socket.cpp b/src/sender/tcp_socket.cpp index 121538d..8c0d4c4 100644 --- a/src/sender/tcp_socket.cpp +++ b/src/sender/tcp_socket.cpp @@ -23,7 +23,7 @@ TcpSocket::~TcpSocket() std::cout << "Socket closed.\n"; } -int TcpSocket::listenForLocalConnection() +bool TcpSocket::listenForLocalConnection() { int tempSockId = 0; @@ -35,7 +35,7 @@ int TcpSocket::listenForLocalConnection() if (tempSockId < 0) { std::cout << "Could not create socket!\n"; // SOCK_STREAM invokes TCP, SOCK_DGRAM would invoke UDP - return 0; + return false; } memset(reinterpret_cast(&remaddr), 0, sizeof(remaddr)); @@ -47,24 +47,25 @@ int TcpSocket::listenForLocalConnection() if (bind(tempSockId, reinterpret_cast(&remaddr), sizeof(remaddr)) < 0) { std::cout << "Error on socket binding!\n"; - return 0; + return false; } - listen(tempSockId, 5); + const auto maximumQueueLength = 5; + listen(tempSockId, maximumQueueLength); clilen = sizeof(cliAddr); std::cout << "Waiting for TCP connection...\n"; sockID = accept(tempSockId, reinterpret_cast(&cliAddr), &clilen); if (sockID < 0) { std::cout << "Error on accepting TCP connection!\n"; - return 0; + return false; } // Temporary socket not needed anymore, closing close(tempSockId); std::cout << "Established TCP connection!\n"; - return 1; + return true; } int TcpSocket::send(unsigned char* addr, int len) const diff --git a/src/sender/tcp_socket.hpp b/src/sender/tcp_socket.hpp index dc57dc0..b7f438c 100644 --- a/src/sender/tcp_socket.hpp +++ b/src/sender/tcp_socket.hpp @@ -13,8 +13,8 @@ class TcpSocket TcpSocket() {}; ~TcpSocket(); - // Listens for a TCP connection on a local port returns 1 on successful connection, 0 on error - int listenForLocalConnection(); + // Listens for a TCP connection on a local port returns true on successful connection, false on error + bool listenForLocalConnection(); int send(unsigned char*, int) const; }; diff --git a/src/sender/usb_camera_frame_grabber.cpp b/src/sender/usb_camera_frame_grabber.cpp index a4d3626..ed2d98a 100644 --- a/src/sender/usb_camera_frame_grabber.cpp +++ b/src/sender/usb_camera_frame_grabber.cpp @@ -26,10 +26,12 @@ void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgR cap.set(cv::CAP_PROP_FRAME_WIDTH, params->width); cap.set(cv::CAP_PROP_FRAME_HEIGHT, params->height); cap.set(cv::CAP_PROP_FPS, params->fps); // This can control fps. But only up to limit given by cam. - cap.set(cv::CAP_PROP_AUTO_EXPOSURE, 0.25); // This value gives manual exposure control - cap.set(cv::CAP_PROP_EXPOSURE, 0.0835); + const auto autoExposure = 0.25; // This value gives manual exposure control for many cameras, but might differ for your camera + cap.set(cv::CAP_PROP_AUTO_EXPOSURE, autoExposure); // An exposure value of 0.0835 seems to enable the highest frame rate on a Logitech C922. // Might differ for your camera. In general, the range for exposure should be [0,1] + const auto exposureValue = 0.0835; + cap.set(cv::CAP_PROP_EXPOSURE, exposureValue); cv::Mat frame; diff --git a/src/sender/video_streamer_tcp.cpp b/src/sender/video_streamer_tcp.cpp index 009014c..803fb6a 100644 --- a/src/sender/video_streamer_tcp.cpp +++ b/src/sender/video_streamer_tcp.cpp @@ -20,8 +20,8 @@ #include "tcp_socket.hpp" // Parses the command line arguments and sets up the camera and encoder objects -// Returns 1 on success, 0 on failure -int argumentParser(int argc, char** argv, TcpSocket* sock, CameraParameters* camParams, Encoder* enc); +// Returns true on success, false on failure +bool parseArguments(int argc, char** argv, TcpSocket* sock, CameraParameters* camParams, Encoder* enc); int main(int argc, char* argv[]) { @@ -39,8 +39,7 @@ int main(int argc, char* argv[]) Encoder enc; TcpSocket sock; - // Parse command line arguments - if (!argumentParser(argc, argv, &sock, &camParams, &enc)) + if (!parseArguments(argc, argv, &sock, &camParams, &enc)) { return -1; } @@ -48,7 +47,6 @@ int main(int argc, char* argv[]) // Allocate memory for raw image auto* img = static_cast(malloc(static_cast(camParams.width) * camParams.height * 3)); - // Listen for connection request if (!sock.listenForLocalConnection()) { return -1; @@ -69,7 +67,8 @@ int main(int argc, char* argv[]) } while ((!imgReady) && !progEnd) { - std::this_thread::sleep_for(std::chrono::microseconds(10)); + const auto sleepDuration = 10; + std::this_thread::sleep_for(std::chrono::microseconds(sleepDuration)); } if (progEnd) { @@ -92,7 +91,8 @@ int main(int argc, char* argv[]) { // Video receiver shut down connection, quit this program progEnd = true; - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + const auto tcpClosureTimeout = 100; + std::this_thread::sleep_for(std::chrono::milliseconds(tcpClosureTimeout)); std::cout << "\n\nClient closed TCP connection. Closing program.\n\n"; break; } @@ -103,12 +103,14 @@ int main(int argc, char* argv[]) timekeeper = std::chrono::high_resolution_clock::now(); // Put info to terminal - if ((framecounter % (static_cast(camParams.fps) / 5)) == 0) + const auto updateFrequency = 5; + if ((framecounter % (static_cast(camParams.fps) / updateFrequency)) == 0) { + const auto millisecondsToSeconds = 1000; std::cout << "\rt_enc=" << std::chrono::duration_cast(encEnd - encStart).count() << "us, fs=" << frameSize << "b, sb=" << sentbytesFrame << "b, fps=" - << (static_cast(framecounter) * 1000) / + << (static_cast(framecounter) * millisecondsToSeconds) / static_cast(std::chrono::duration_cast(timekeeper - overallStart).count()) << "Hz. " << std::flush; } @@ -120,7 +122,7 @@ int main(int argc, char* argv[]) return 0; } -int argumentParser(int argc, char* argv[], TcpSocket* sock, CameraParameters* camParams, Encoder* enc) +bool parseArguments(int argc, char* argv[], TcpSocket* sock, CameraParameters* camParams, Encoder* enc) { std::stringstream ss; @@ -148,15 +150,16 @@ int argumentParser(int argc, char* argv[], TcpSocket* sock, CameraParameters* ca break; default: std::cout << msg << std::endl; - return 0; + return false; } // Get values from config file *sock = TcpSocket(conf["port"]); - camParams->eye = static_cast(malloc(99)); + const auto bytesForEyeName = 99; + camParams->eye = static_cast(malloc(bytesForEyeName)); temp = static_cast(conf["camera.name"]); - temp.copy(camParams->eye, 99); + temp.copy(camParams->eye, bytesForEyeName); camParams->width = 2 * (static_cast(conf["camera.width"])) / 2; camParams->height = 4 * (static_cast(conf["camera.height"])) / 4; camParams->sensor_width = conf["camera.sensor_width"]; @@ -169,9 +172,12 @@ int argumentParser(int argc, char* argv[], TcpSocket* sock, CameraParameters* ca *enc = Encoder(conf["camera.width"], conf["camera.height"], conf["video.width"], conf["video.height"], conf["fps"]); // Computing remaining values - camParams->t_exp = static_cast(1000000 / (camParams->fps * 1.005)); - camParams->xoff = 16 * ((camParams->sensor_width - camParams->width) / (16 * 2)); - camParams->yoff = 16 * ((camParams->sensor_height - camParams->height) / (16 * 2)); - - return 1; + const auto microsecondsToSeconds = 1000000; + const auto exposureTimeFactor = 1.005; // Empirically determined factor to get the actual exposure time slightly below the frame time, to avoid dropped frames + const auto offsetGridSize = 16; + camParams->t_exp = static_cast(microsecondsToSeconds / (camParams->fps * exposureTimeFactor)); + camParams->xoff = offsetGridSize * ((camParams->sensor_width - camParams->width) / (offsetGridSize * 2)); + camParams->yoff = offsetGridSize * ((camParams->sensor_height - camParams->height) / (offsetGridSize * 2)); + + return true; } From 196c00477e062ae3a64f69f67508594b1e60ca0e Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 10:24:42 +0200 Subject: [PATCH 4/7] Fix clang-format --- .pre-commit-config.yaml | 2 -- src/receiver/decoder.cpp | 41 +++++++++++++++---------- src/receiver/decoder.h | 3 +- src/receiver/video_player.cpp | 13 ++++---- src/sender/encoder.hpp | 8 ++--- src/sender/tcp_socket.cpp | 10 ++---- src/sender/usb_camera_frame_grabber.cpp | 6 ++-- src/sender/video_streamer_tcp.cpp | 25 +++++++-------- 8 files changed, 55 insertions(+), 53 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f4fbe9e..a9ae795 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,8 +15,6 @@ repos: - id: check-executables-have-shebangs - id: check-shebang-scripts-are-executable - id: check-symlinks - - id: check-yaml - exclude: ".clang-format" - id: debug-statements - id: destroyed-symlinks - id: detect-private-key diff --git a/src/receiver/decoder.cpp b/src/receiver/decoder.cpp index 53c14b9..422c4e7 100644 --- a/src/receiver/decoder.cpp +++ b/src/receiver/decoder.cpp @@ -5,16 +5,15 @@ #include // Socket includes +#include + #include +#include //File control definitions #include #include #include -#include - -#include - -#include //File control definitions #include //POSIX terminal control definitions +#include // #define ARDUINO_MSMT constexpr int kMsgLength = 30; @@ -160,10 +159,19 @@ void Decoder(const char* videoAddress, uint8_t** argbRaw, bool* /*newImg*/) //---------------------------- CSP conversion initialization //------------------------------------- //--------------------------------------------------------------------------------------------------- - swsCtxYuv2Bgra = sws_getContext( - width, height, AV_PIX_FMT_YUV420P, targetWidth, targetHeight, AV_PIX_FMT_RGBA, SWS_BILINEAR, nullptr, nullptr, nullptr); - - auto* argbData = static_cast(malloc(static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t))); + swsCtxYuv2Bgra = sws_getContext(width, + height, + AV_PIX_FMT_YUV420P, + targetWidth, + targetHeight, + AV_PIX_FMT_RGBA, + SWS_BILINEAR, + nullptr, + nullptr, + nullptr); + + auto* argbData = + static_cast(malloc(static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t))); argbRaw[0] = argbData; argbRaw[1] = argbData + static_cast(targetWidth) * targetHeight; argbRaw[2] = argbData + static_cast(targetWidth) * targetHeight * 2; @@ -252,14 +260,13 @@ void Decoder(const char* videoAddress, uint8_t** argbRaw, bool* /*newImg*/) constexpr int kPrintInterval = 5; if (framecounter % (kFps / kPrintInterval) == 0) { - std::cout - << "\r" - << "rb=" << pkt->size << "B, " - << "t_dec=" - << std::chrono::duration_cast(decodingEnd - decodingStart).count() - << "us, t_cc=" - << std::chrono::duration_cast(ccEnd - decodingEnd).count() - << std::flush; + std::cout << "\r" + << "rb=" << pkt->size << "B, " + << "t_dec=" + << std::chrono::duration_cast(decodingEnd - decodingStart).count() + << "us, t_cc=" + << std::chrono::duration_cast(ccEnd - decodingEnd).count() + << std::flush; } #endif } diff --git a/src/receiver/decoder.h b/src/receiver/decoder.h index 679248c..591737e 100644 --- a/src/receiver/decoder.h +++ b/src/receiver/decoder.h @@ -2,10 +2,9 @@ #define DECODER_H_ // System includes +#include #include #include - -#include #include #include #include diff --git a/src/receiver/video_player.cpp b/src/receiver/video_player.cpp index 350d00a..9a34ab1 100644 --- a/src/receiver/video_player.cpp +++ b/src/receiver/video_player.cpp @@ -19,10 +19,10 @@ bool video = true; bool fullscreen = false; int vsync = 1; const int targetWidth = 1280, targetHeight = 720; -int height = targetHeight, width = targetWidth; // will be overwritten by the video dimensions in the decoder; definition here - // necessary if no video is used +int height = targetHeight, width = targetWidth; // will be overwritten by the video dimensions in the decoder; + // definition here necessary if no video is used -bool readyToQuit = false; // Quits all threads +bool readyToQuit = false; // Quits all threads uint8_t** argbSrc = static_cast(malloc(sizeof(uint8_t*) * 4)); // Pointer to the decoded image bool newImage = false; // Set to true by the decoder when it decodes a frame. Set to false after oculus as copied a decoded frame @@ -40,7 +40,7 @@ int main(int argc, char* argv[]) else { std::cout << "Decoding video from default url:" << videoUrl << ". For other sources, use e.g. '" << argv[0] - << " tcp://10.152.4.207:5000'\n"; + << " tcp://10.152.4.207:5000'\n"; } // SDL Inits @@ -58,7 +58,8 @@ int main(int argc, char* argv[]) auto* event = new SDL_Event(); - auto* pixelData = static_cast(malloc(static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t))); + auto* pixelData = + static_cast(malloc(static_cast(targetWidth) * targetHeight * 4 * sizeof(uint8_t))); uint8_t** pixels = nullptr; int* argbStride = nullptr; pixels = static_cast(malloc(sizeof(uint8_t*) * 4)); @@ -78,7 +79,7 @@ int main(int argc, char* argv[]) } //------------------ UDP port setup --------------- - struct sockaddr_in remaddr {}; + struct sockaddr_in remaddr{}; int sockFd = 0; const int slen = sizeof(remaddr); diff --git a/src/sender/encoder.hpp b/src/sender/encoder.hpp index c3d4574..4fe72eb 100644 --- a/src/sender/encoder.hpp +++ b/src/sender/encoder.hpp @@ -22,12 +22,12 @@ class Encoder int framecounter = 0; int nheader = 0; x264_t* enc = nullptr; - x264_param_t prms {}; - x264_picture_t pic_in {}; - x264_picture_t pic_out {}; + x264_param_t prms{}; + x264_picture_t pic_in{}; + x264_picture_t pic_out{}; struct SwsContext* sws = nullptr; - AVFrame picRaw {}; + AVFrame picRaw{}; AVPixelFormat camPixelFmt = AV_PIX_FMT_BGR24; public: diff --git a/src/sender/tcp_socket.cpp b/src/sender/tcp_socket.cpp index 8c0d4c4..49bd23e 100644 --- a/src/sender/tcp_socket.cpp +++ b/src/sender/tcp_socket.cpp @@ -3,7 +3,6 @@ #include #include #include - #include #include @@ -12,10 +11,7 @@ #include #include -TcpSocket::TcpSocket(int port) - : portID(port), sockID(0) -{ -} +TcpSocket::TcpSocket(int port) : portID(port), sockID(0) {} TcpSocket::~TcpSocket() { @@ -27,8 +23,8 @@ bool TcpSocket::listenForLocalConnection() { int tempSockId = 0; - struct sockaddr_in remaddr {}; - struct sockaddr_in cliAddr {}; + struct sockaddr_in remaddr{}; + struct sockaddr_in cliAddr{}; socklen_t clilen = 0; tempSockId = socket(AF_INET, SOCK_STREAM, 0); diff --git a/src/sender/usb_camera_frame_grabber.cpp b/src/sender/usb_camera_frame_grabber.cpp index ed2d98a..76d3cea 100644 --- a/src/sender/usb_camera_frame_grabber.cpp +++ b/src/sender/usb_camera_frame_grabber.cpp @@ -2,7 +2,6 @@ #include #include - #include #include @@ -25,8 +24,9 @@ void cameraFrameGrabber(CameraParameters* params, unsigned char* img, bool* imgR cap.set(cv::CAP_PROP_FOURCC, cv::VideoWriter::fourcc('M', 'J', 'P', 'G')); cap.set(cv::CAP_PROP_FRAME_WIDTH, params->width); cap.set(cv::CAP_PROP_FRAME_HEIGHT, params->height); - cap.set(cv::CAP_PROP_FPS, params->fps); // This can control fps. But only up to limit given by cam. - const auto autoExposure = 0.25; // This value gives manual exposure control for many cameras, but might differ for your camera + cap.set(cv::CAP_PROP_FPS, params->fps); // This can control fps. But only up to limit given by cam. + const auto autoExposure = + 0.25; // This value gives manual exposure control for many cameras, but might differ for your camera cap.set(cv::CAP_PROP_AUTO_EXPOSURE, autoExposure); // An exposure value of 0.0835 seems to enable the highest frame rate on a Logitech C922. // Might differ for your camera. In general, the range for exposure should be [0,1] diff --git a/src/sender/video_streamer_tcp.cpp b/src/sender/video_streamer_tcp.cpp index 803fb6a..c6728e3 100644 --- a/src/sender/video_streamer_tcp.cpp +++ b/src/sender/video_streamer_tcp.cpp @@ -4,11 +4,10 @@ #include "usb_camera_frame_grabber.hpp" #endif +#include #include #include #include - -#include #include #include #include @@ -26,8 +25,8 @@ bool parseArguments(int argc, char** argv, TcpSocket* sock, CameraParameters* ca int main(int argc, char* argv[]) { // Variable initializations - bool progEnd = false; // Shared between threads to signal program shutdown - bool imgReady = false; // Synchronizes cameraFrameGrabber and encoder + bool progEnd = false; // Shared between threads to signal program shutdown + bool imgReady = false; // Synchronizes cameraFrameGrabber and encoder int framecounter = 0; std::chrono::time_point encStart; std::chrono::time_point encEnd; @@ -35,7 +34,7 @@ int main(int argc, char* argv[]) std::chrono::time_point overallStart; // Main objects: camera parameters, encoder and tcp Socket - CameraParameters camParams {}; + CameraParameters camParams{}; Encoder enc; TcpSocket sock; @@ -107,12 +106,13 @@ int main(int argc, char* argv[]) if ((framecounter % (static_cast(camParams.fps) / updateFrequency)) == 0) { const auto millisecondsToSeconds = 1000; - std::cout << "\rt_enc=" - << std::chrono::duration_cast(encEnd - encStart).count() - << "us, fs=" << frameSize << "b, sb=" << sentbytesFrame << "b, fps=" - << (static_cast(framecounter) * millisecondsToSeconds) / - static_cast(std::chrono::duration_cast(timekeeper - overallStart).count()) - << "Hz. " << std::flush; + std::cout + << "\rt_enc=" << std::chrono::duration_cast(encEnd - encStart).count() + << "us, fs=" << frameSize << "b, sb=" << sentbytesFrame << "b, fps=" + << (static_cast(framecounter) * millisecondsToSeconds) / + static_cast( + std::chrono::duration_cast(timekeeper - overallStart).count()) + << "Hz. " << std::flush; } framecounter++; @@ -173,7 +173,8 @@ bool parseArguments(int argc, char* argv[], TcpSocket* sock, CameraParameters* c // Computing remaining values const auto microsecondsToSeconds = 1000000; - const auto exposureTimeFactor = 1.005; // Empirically determined factor to get the actual exposure time slightly below the frame time, to avoid dropped frames + const auto exposureTimeFactor = 1.005; // Empirically determined factor to get the actual exposure time slightly + // below the frame time, to avoid dropped frames const auto offsetGridSize = 16; camParams->t_exp = static_cast(microsecondsToSeconds / (camParams->fps * exposureTimeFactor)); camParams->xoff = offsetGridSize * ((camParams->sensor_width - camParams->width) / (offsetGridSize * 2)); From ef580a17607b2f11cc3d7ccd4389fdcdd4c2727f Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 10:24:59 +0200 Subject: [PATCH 5/7] Ignore build dir --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 722d5e7..875eed3 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ .vscode +build/ From a1ed4ed33e1a84727f7c21c165aca37abd2f5296 Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 10:25:35 +0200 Subject: [PATCH 6/7] Build in parallel --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 58475fc..9526a6b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,4 +30,4 @@ jobs: - name: configure run: mkdir -p build && cd build && cmake .. -DENABLE_CLANG_TIDY=ON - name: build - run: cd build && cmake --build . + run: cd build && cmake --build . -j $(nproc) From 87a8e396984ae77a4393056062544cf7c8421514 Mon Sep 17 00:00:00 2001 From: Chris Bachhuber Date: Tue, 28 Apr 2026 10:29:45 +0200 Subject: [PATCH 7/7] Distinguish OS --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9526a6b..8e71c17 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -21,12 +21,12 @@ jobs: uses: awalsh128/cache-apt-pkgs-action@latest with: packages: build-essential cmake ffmpeg libx264-dev x264 libopencv-dev libva-dev libsdl2-dev - version: 1.0 # bump this if you change the package list to invalidate the cache + version: ${{ matrix.os }}-1.0 # bump this if you change the package list to invalidate the cache - name: cache development apt packages uses: awalsh128/cache-apt-pkgs-action@latest with: packages: clang-tidy - version: 1.0 # bump this if you change the package list to invalidate the cache + version: ${{ matrix.os }}-1.0 # bump this if you change the package list to invalidate the cache - name: configure run: mkdir -p build && cd build && cmake .. -DENABLE_CLANG_TIDY=ON - name: build