code review suggestions

tools/ddprobe.cpp

@@ -9,10 +9,13 @@
 #include <iostream>
 #include <locale>
 #include <string>
+#include <wrl.h>
 
 #include "src/utility.h"
 
 using namespace std::literals;
+using Microsoft::WRL::ComPtr;
+
 namespace dxgi {
   template <class T>
   void
@@ -70,72 +73,80 @@ syncThreadDesktop() {
 }
 
 /**
- * @brief Determines whether a given frame is entirely black by checking every pixel.
+ * @brief Checks whether a given frame is entirely dark by evaluating the RGB values of each pixel.
  *
- * This function checks if the provided frame is entirely black by inspecting each pixel in both the x and y dimensions. It inspects the RGB channels of each pixel and compares them against a specified black threshold. If any pixel's RGB values exceed this threshold, the frame is considered not black, and the function returns `false`. Otherwise, if all pixels are below the threshold, the function returns `true`.
+ * This function determines if the provided frame is completely dark by analyzing the RGB values of every pixel.
+ * It iterates over all pixels in the frame and compares each pixel's RGB channels to a defined darkness threshold.
+ * If any pixel's RGB values exceed this threshold, the function concludes that the frame is not entirely dark and returns `false`.
+ * If all pixels are below the threshold, indicating a completely dark frame, the function returns `true`.
  *
- * @param mappedResource A reference to a `D3D11_MAPPED_SUBRESOURCE` structure that contains the mapped subresource data of the frame to be analyzed.
+ * @param mappedResource A reference to a `D3D11_MAPPED_SUBRESOURCE` structure containing the mapped subresource data of the frame to be analyzed.
- * @param frameDesc A reference to a `D3D11_TEXTURE2D_DESC` structure that describes the texture properties, including width and height.
+ * @param frameDesc A reference to a `D3D11_TEXTURE2D_DESC` structure describing the texture properties, including width and height.
- * @param blackThreshold A floating-point value representing the threshold above which a pixel's RGB channels are considered non-black. The value ranges from 0.0f to 1.0f, with a default value of 0.1f.
+ * @param darknessThreshold A floating-point value representing the threshold above which a pixel's RGB values are considered non-dark. The value ranges from 0.0f to 1.0f, with a default value of 0.1f.
- * @return bool Returns `true` if the frame is determined to be black, otherwise returns `false`.
+ * @return bool Returns `true` if the frame is determined to be entirely dark, otherwise returns `false`.
  */
 bool
-isFrameBlack(const D3D11_MAPPED_SUBRESOURCE &mappedResource, const D3D11_TEXTURE2D_DESC &frameDesc, float blackThreshold = 0.1f) {
+is_valid_frame(const D3D11_MAPPED_SUBRESOURCE &mappedResource, const D3D11_TEXTURE2D_DESC &frameDesc, float darknessThreshold = 0.1f) {
   const uint8_t *pixels = static_cast<const uint8_t *>(mappedResource.pData);
-  const int bytesPerPixel = 4;  // Assuming RGBA format
+  const int bytesPerPixel = 4;  // (8 bits per channel, excluding alpha). Factoring HDR is not needed because it doesn't cause black levels to raise enough to be a concern.
   const int stride = mappedResource.RowPitch;
   const int width = frameDesc.Width;
   const int height = frameDesc.Height;
 
-  // Convert the threshold to an integer value for comparison
+  // Convert the darkness threshold to an integer value for comparison
-  const int threshold = static_cast<int>(blackThreshold * 255);
+  const int threshold = static_cast<int>(darknessThreshold * 255);
 
-  // Loop through every pixel
+  // Iterate over each pixel in the frame
   for (int y = 0; y < height; ++y) {
     for (int x = 0; x < width; ++x) {
       const uint8_t *pixel = pixels + y * stride + x * bytesPerPixel;
-      // Check if any channel (R, G, B) is significantly above black
+      // Check if any RGB channel exceeds the darkness threshold
       if (pixel[0] > threshold || pixel[1] > threshold || pixel[2] > threshold) {
-        return false;
+        // Frame is not dark
+        return true;
       }
     }
   }
-  return true;
+  // Frame is entirely dark
+  return false;
 }
 
 /**
- * @brief Attempts to capture and verify the contents of up to 10 consecutive frames from a DXGI output duplication.
+ * @brief Captures and verifies the contents of up to 10 consecutive frames from a DXGI output duplication.
  *
- * This function tries to acquire the next frame from a provided DXGI output duplication object (`dup`) and inspects its content to determine if the frame is not completely black. If a non-black frame is found within the 10 attempts, the function returns `S_OK`. If all 10 frames are black, it returns `S_FALSE`, indicating that the capture might not be functioning properly. In case of any failure during the process, the appropriate `HRESULT` error code is returned.
+ * This function attempts to acquire and analyze up to 10 frames from a DXGI output duplication object (`dup`).
+ * It checks if each frame is non-empty (not entirely dark) by using the `is_valid_frame` function.
+ * If any non-empty frame is found, the function returns `S_OK`.
+ * If all 10 frames are empty, it returns `S_FALSE`, suggesting potential issues with the capture process.
+ * If any error occurs during the frame acquisition or analysis process, the corresponding `HRESULT` error code is returned.
  *
  * @param dup A reference to the DXGI output duplication object (`dxgi::dup_t&`) used to acquire frames.
- * @param device A pointer to the ID3D11Device interface that represents the device associated with the Direct3D context.
+ * @param device A ComPtr to the ID3D11Device interface representing the device associated with the Direct3D context.
- * @return HRESULT Returns `S_OK` if a non-black frame is captured successfully, `S_FALSE` if all frames are black, or an error code if a failure occurs during the process.
+ * @return HRESULT Returns `S_OK` if a non-empty frame is captured successfully, `S_FALSE` if all frames are empty, or an error code if any failure occurs during the process.
- *
- * Possible return values:
- * - `S_OK`: A non-black frame was captured, indicating capture was successful.
- * - `S_FALSE`: All 10 frames were black, indicating potential capture issues.
- * - `E_FAIL`, `DXGI_ERROR_*`, or other DirectX HRESULT error codes in case of specific failures during frame capture, texture creation, or resource mapping.
  */
 HRESULT
-test_frame_capture(dxgi::dup_t &dup, ID3D11Device *device) {
+test_frame_capture(dxgi::dup_t &dup, ComPtr<ID3D11Device> device) {
   for (int i = 0; i < 10; ++i) {
-    std::cout << "Attempting to acquire the next frame (" << i + 1 << "/10)..." << std::endl;
+    std::cout << "Attempting to acquire frame " << (i + 1) << " of 10..." << std::endl;
-    IDXGIResource *frameResource = nullptr;
+
+    ComPtr<IDXGIResource> frameResource;
     DXGI_OUTDUPL_FRAME_INFO frameInfo;
     HRESULT status = dup->AcquireNextFrame(500, &frameInfo, &frameResource);
+    auto release_frame = util::fail_guard([&dup]() {
+      dup->ReleaseFrame();
+    });
+
     if (FAILED(status)) {
-      std::cout << "Failed to acquire next frame [0x" << std::hex << status << "]" << std::endl;
+      std::cout << "Error: Failed to acquire next frame [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
       return status;
     }
 
     std::cout << "Frame acquired successfully." << std::endl;
 
-    ID3D11Texture2D *frameTexture = nullptr;
+    ComPtr<ID3D11Texture2D> frameTexture;
-    status = frameResource->QueryInterface(__uuidof(ID3D11Texture2D), (void **) &frameTexture);
+    status = frameResource->QueryInterface(__uuidof(ID3D11Texture2D), reinterpret_cast<void **>(frameTexture.GetAddressOf()));
-    frameResource->Release();
     if (FAILED(status)) {
-      std::cout << "Failed to query texture interface from frame resource [0x" << std::hex << status << "]" << std::endl;
+      std::cout << "Error: Failed to query texture interface from frame resource [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
       return status;
     }
 
@@ -146,51 +157,37 @@ test_frame_capture(dxgi::dup_t &dup, ID3D11Device *device) {
     frameDesc.BindFlags = 0;
     frameDesc.MiscFlags = 0;
 
-    ID3D11Texture2D *stagingTexture = nullptr;
+    ComPtr<ID3D11Texture2D> stagingTexture;
     status = device->CreateTexture2D(&frameDesc, nullptr, &stagingTexture);
     if (FAILED(status)) {
-      std::cout << "Failed to create staging texture [0x" << std::hex << status << "]" << std::endl;
+      std::cout << "Error: Failed to create staging texture [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
-      frameTexture->Release();
       return status;
     }
 
-    ID3D11DeviceContext *context = nullptr;
+    ComPtr<ID3D11DeviceContext> context;
     device->GetImmediateContext(&context);
-    context->CopyResource(stagingTexture, frameTexture);
+    context->CopyResource(stagingTexture.Get(), frameTexture.Get());
-    frameTexture->Release();
 
     D3D11_MAPPED_SUBRESOURCE mappedResource;
-    status = context->Map(stagingTexture, 0, D3D11_MAP_READ, 0, &mappedResource);
+    status = context->Map(stagingTexture.Get(), 0, D3D11_MAP_READ, 0, &mappedResource);
-    if (SUCCEEDED(status)) {
+    if (FAILED(status)) {
-      std::cout << "Verifying if frame is not empty" << std::endl;
+      std::cout << "Error: Failed to map the staging texture for inspection [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
-
-      if (!isFrameBlack(mappedResource, frameDesc)) {
-        std::cout << "Frame " << i + 1 << " is not empty!" << std::endl;
-        context->Unmap(stagingTexture, 0);
-        stagingTexture->Release();
-        context->Release();
-        dup->ReleaseFrame();
-        return S_OK;
-      }
-      context->Unmap(stagingTexture, 0);
-    }
-    else {
-      std::cout << "Failed to map the staging texture for inspection [0x" << std::hex << status << "]" << std::endl;
-      stagingTexture->Release();
-      context->Release();
       return status;
     }
 
-    stagingTexture->Release();
+    if (is_valid_frame(mappedResource, frameDesc)) {
-    context->Release();
+      std::cout << "Frame " << (i + 1) << " is non-empty (contains visible content)." << std::endl;
-    std::cout << "Releasing the frame..." << std::endl;
+      context->Unmap(stagingTexture.Get(), 0);
-    dup->ReleaseFrame();
+      return S_OK;
+    }
+
+    std::cout << "Frame " << (i + 1) << " is empty (no visible content)." << std::endl;
+    context->Unmap(stagingTexture.Get(), 0);
   }
 
-  // If all frames are black, then we can assume the capture isn't working properly.
+  // All frames were empty, indicating potential capture issues.
   return S_FALSE;
 }
-
 HRESULT
 test_dxgi_duplication(dxgi::adapter_t &adapter, dxgi::output_t &output) {
   D3D_FEATURE_LEVEL featureLevels[] = {
@@ -232,11 +229,12 @@ test_dxgi_duplication(dxgi::adapter_t &adapter, dxgi::output_t &output) {
 
   // Attempt to duplicate the output
   dxgi::dup_t dup;
-  HRESULT result = output1->DuplicateOutput(static_cast<IUnknown *>(device.get()), &dup);
+  ComPtr<ID3D11Device> device_ptr(device.get());
+  HRESULT result = output1->DuplicateOutput(device_ptr.Get(), &dup);
 
   if (SUCCEEDED(result)) {
     // If duplication is successful, test frame capture
-    HRESULT captureResult = test_frame_capture(dup, device.get());
+    HRESULT captureResult = test_frame_capture(dup, device_ptr.Get());
     if (SUCCEEDED(captureResult)) {
       return S_OK;
     }