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 <[email protected]>
Commit-Queue: Ilya Nikolaevskiy <[email protected]>
Reviewed-by: Per Kjellander <[email protected]>
Cr-Commit-Position: refs/heads/main@{#40290}

Author: Pehrsons

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<uint8_t*>(_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;
 }

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;