From 823c70209e619c8f278e8abae35c9e7816579eff Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Mon, 22 May 2023 10:52:17 +0200 Subject: [PATCH] In VideoCaptureV4L2 use requested/configured capability VideoCaptureV4L2 has some members that are set in StartCapture during configuration of the device, but later accessed both on the capture thread and on the api thread (same as StartCapture) in CaptureSettings(), which can be called at any time. This is safe because they are written only on the api thread when the capture thread is not running. However, there is no helper class that separates the read and write modes to allow annotations and static analysis of the thread access of these members. This commit allows annotations to be added by making VideoCaptureV4L2 use: - VideoCaptureImpl::_requestedCapability for storing those members requested through StartCapture(), to allow access on the api thread through CaptureSettings(). - A new member configured_capability_ to replace those members mentioned in the first paragraph above. Writes to it happen only in StartCapture() and StopCapture(), while reads happen exclusively on the capture thread in between. Bug: webrtc:15181 Change-Id: I27e0f578e6ac2a2e6b0e34fbabfe4f743b296321 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306122 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Per Kjellander Cr-Commit-Position: refs/heads/main@{#40290} --- .../video_capture/linux/video_capture_v4l2.cc | 63 +++++++++---------- .../video_capture/linux/video_capture_v4l2.h | 5 +- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/modules/video_capture/linux/video_capture_v4l2.cc b/modules/video_capture/linux/video_capture_v4l2.cc index bb543a583f..752bb875f5 100644 --- a/modules/video_capture/linux/video_capture_v4l2.cc +++ b/modules/video_capture/linux/video_capture_v4l2.cc @@ -54,11 +54,7 @@ VideoCaptureModuleV4L2::VideoCaptureModuleV4L2() _deviceId(-1), _deviceFd(-1), _buffersAllocatedByDevice(-1), - _currentWidth(-1), - _currentHeight(-1), - _currentFrameRate(-1), _captureStarted(false), - _captureVideoType(VideoType::kI420), _pool(NULL) {} int32_t VideoCaptureModuleV4L2::Init(const char* deviceUniqueIdUTF8) { @@ -111,15 +107,17 @@ VideoCaptureModuleV4L2::~VideoCaptureModuleV4L2() { int32_t VideoCaptureModuleV4L2::StartCapture( const VideoCaptureCapability& capability) { if (_captureStarted) { - if (capability.width == _currentWidth && - capability.height == _currentHeight && - _captureVideoType == capability.videoType) { + if (capability == _requestedCapability) { return 0; } else { StopCapture(); } } + // Set a baseline of configured parameters. It is updated here during + // configuration, then read from the capture thread. + configured_capability_ = capability; + MutexLock lock(&capture_lock_); // first open /dev/video device char device[20]; @@ -189,32 +187,32 @@ int32_t VideoCaptureModuleV4L2::StartCapture( video_fmt.fmt.pix.pixelformat = fmts[fmtsIdx]; if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) - _captureVideoType = VideoType::kYUY2; + configured_capability_.videoType = VideoType::kYUY2; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_YUV420) - _captureVideoType = VideoType::kI420; + configured_capability_.videoType = VideoType::kI420; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_YVU420) - _captureVideoType = VideoType::kYV12; + configured_capability_.videoType = VideoType::kYV12; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_UYVY) - _captureVideoType = VideoType::kUYVY; + configured_capability_.videoType = VideoType::kUYVY; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_NV12) - _captureVideoType = VideoType::kNV12; + configured_capability_.videoType = VideoType::kNV12; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_BGR24) - _captureVideoType = VideoType::kRGB24; + configured_capability_.videoType = VideoType::kRGB24; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_RGB24) - _captureVideoType = VideoType::kBGR24; + configured_capability_.videoType = VideoType::kBGR24; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_RGB565) - _captureVideoType = VideoType::kRGB565; + configured_capability_.videoType = VideoType::kRGB565; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_ABGR32 || video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_BGR32) - _captureVideoType = VideoType::kARGB; + configured_capability_.videoType = VideoType::kARGB; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_ARGB32 || video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_RGB32) - _captureVideoType = VideoType::kBGRA; + configured_capability_.videoType = VideoType::kBGRA; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_RGBA32) - _captureVideoType = VideoType::kABGR; + configured_capability_.videoType = VideoType::kABGR; else if (video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_MJPEG || video_fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_JPEG) - _captureVideoType = VideoType::kMJPEG; + configured_capability_.videoType = VideoType::kMJPEG; else RTC_DCHECK_NOTREACHED(); @@ -225,8 +223,8 @@ int32_t VideoCaptureModuleV4L2::StartCapture( } // initialize current width and height - _currentWidth = video_fmt.fmt.pix.width; - _currentHeight = video_fmt.fmt.pix.height; + configured_capability_.width = video_fmt.fmt.pix.width; + configured_capability_.height = video_fmt.fmt.pix.height; // Trying to set frame rate, before check driver capability. bool driver_framerate_support = true; @@ -248,18 +246,17 @@ int32_t VideoCaptureModuleV4L2::StartCapture( if (ioctl(_deviceFd, VIDIOC_S_PARM, &streamparms) < 0) { RTC_LOG(LS_INFO) << "Failed to set the framerate. errno=" << errno; driver_framerate_support = false; - } else { - _currentFrameRate = capability.maxFPS; } } } // If driver doesn't support framerate control, need to hardcode. // Hardcoding the value based on the frame size. if (!driver_framerate_support) { - if (_currentWidth >= 800 && _captureVideoType != VideoType::kMJPEG) { - _currentFrameRate = 15; + if (configured_capability_.width >= 800 && + configured_capability_.videoType != VideoType::kMJPEG) { + configured_capability_.maxFPS = 15; } else { - _currentFrameRate = 30; + configured_capability_.maxFPS = 30; } } @@ -276,6 +273,7 @@ int32_t VideoCaptureModuleV4L2::StartCapture( return -1; } + _requestedCapability = capability; _captureStarted = true; // start capture thread; @@ -309,6 +307,8 @@ int32_t VideoCaptureModuleV4L2::StopCapture() { DeAllocateVideoBuffers(); close(_deviceFd); _deviceFd = -1; + + _requestedCapability = configured_capability_ = VideoCaptureCapability(); } return 0; @@ -431,14 +431,10 @@ bool VideoCaptureModuleV4L2::CaptureProcess() { return true; } } - VideoCaptureCapability frameInfo; - frameInfo.width = _currentWidth; - frameInfo.height = _currentHeight; - frameInfo.videoType = _captureVideoType; // convert to to I420 if needed IncomingFrame(reinterpret_cast(_pool[buf.index].start), - buf.bytesused, frameInfo); + buf.bytesused, configured_capability_); // enqueue the buffer again if (ioctl(_deviceFd, VIDIOC_QBUF, &buf) == -1) { RTC_LOG(LS_INFO) << "Failed to enqueue capture buffer"; @@ -451,10 +447,7 @@ bool VideoCaptureModuleV4L2::CaptureProcess() { int32_t VideoCaptureModuleV4L2::CaptureSettings( VideoCaptureCapability& settings) { - settings.width = _currentWidth; - settings.height = _currentHeight; - settings.maxFPS = _currentFrameRate; - settings.videoType = _captureVideoType; + settings = _requestedCapability; return 0; } diff --git a/modules/video_capture/linux/video_capture_v4l2.h b/modules/video_capture/linux/video_capture_v4l2.h index 65e89e2daa..e070e2ff16 100644 --- a/modules/video_capture/linux/video_capture_v4l2.h +++ b/modules/video_capture/linux/video_capture_v4l2.h @@ -48,11 +48,8 @@ class VideoCaptureModuleV4L2 : public VideoCaptureImpl { int32_t _deviceFd; int32_t _buffersAllocatedByDevice; - int32_t _currentWidth; - int32_t _currentHeight; - int32_t _currentFrameRate; + VideoCaptureCapability configured_capability_; bool _captureStarted; - VideoType _captureVideoType; struct Buffer { void* start; size_t length;