PlusToolkit / ndicapi

A common C API for communicating with NDI Polaris and Aurora devices
MIT License
48 stars 32 forks source link

C++ Example - ParallelProbe Errors (Apple clang) #33

Open rajkundu opened 2 years ago

rajkundu commented 2 years ago

Hello! When compiling /Applications/ndiBasicExample.cxx (renamed to .cpp), I encounter the following compilation errors (Apple clang version 14.0.0, x86-64, macOS 12.5):

  1. ndiBasicExample.cpp:30:51: error: variable 'checkDSR' cannot be implicitly captured in a lambda with no capture-default specified
         int errnum = ndiSerialProbe(devName.c_str(),checkDSR);
                                                     ^
  2. ndiBasicExample.cpp:47:13: error: cannot initialize a variable of type 'char *' with an rvalue of type 'const char *'
         char* devicename = ndiSerialDeviceName(i);
               ^            ~~~~~~~~~~~~~~~~~~~~~~
  3. ndiBasicExample.cpp:70:5: error: no matching function for call to 'ParallelProbe'
       ParallelProbe(device,argc > 1 ? argv[1]: 0, checkDSR);
       ^~~~~~~~~~~~~
    ndiBasicExample.cpp:14:6: note: candidate function not viable: requires 2 arguments, but 3 were provided
    bool ParallelProbe(ndicapi*& outDevice, bool checkDSR)
        ^

While I see that ParallelProbe is technically only supposed to be compiled (i.e., supported) on MSVC 1700 (v11.0) or higher, wanted to try to adapt and compile it for my Mac.

rajkundu commented 2 years ago

I think I was able to solve the above issues. However, I am relatively new to C++ and not well-versed in its lambda syntax/std::async/etc. If anyone could quickly glance over this, that would be much appreciated!

  1. It seems that checkDSR needs to be added to the lambda's capture list. So, I changed line 23 as follows:
    std::future<void> result = std::async([i, &deviceNameMutex, &deviceExists]() // old
    std::future<void> result = std::async([i, &deviceNameMutex, &deviceExists, checkDSR]() // new
  2. It seems that devicename needs to be declared as a const char*, not char*. So, I changed line 47 as follows:
    char* devicename = ndiSerialDeviceName(i); // old
    const char* devicename = ndiSerialDeviceName(i); // new
  3. It seems that ParallelProbe is passed a device name argument. However, according to its signature, it doesn't accept any such parameter. So, I changed line 70 as follows:
    ParallelProbe(device,argc > 1 ? argv[1]: 0, checkDSR); // old
    ParallelProbe(device, checkDSR); // new

    I'm not sure if this last fix is correct. While it makes the code compile, I could definitely be missing something.

I'll test this code when I next have access to an NDI camera, and I'll update this thread with the results. In the meantime, it would be great to have someone else check over the above code if possible. Thank you!

rajkundu commented 2 years ago

While not technically a compilation error, I also had to change ParallelProbe as follows to avoid segfaulting:

// Old code
std::string devName;
{
  std::lock_guard<std::mutex> guard(deviceNameMutex);
  devName = std::string(ndiSerialDeviceName(i));
}
int errnum = ndiSerialProbe(devName.c_str(),checkDSR);
if (errnum == NDI_OKAY)
{
  deviceExists[i] = true;
}

// New code (+ minor optimization)
std::string devName;
{
  std::lock_guard<std::mutex> guard(deviceNameMutex);
  const char* name = ndiSerialDeviceName(i);
  if (name == nullptr)
  {
    return;
  }
  devName.assign(name); // use .assign() instead of constructing another std::string object
}
int errnum = ndiSerialProbe(devName.c_str(), checkDSR);
if (errnum == NDI_OKAY)
{
  deviceExists[i] = true;
}

I think the segfaults occurred when NULL/nullptr was passed to the std::string constructor. So, I just checked for null (+ made a minor optimization).

Now ParallelProbe seems to compile and run fine on my Mac – at least with no NDI devices attached. I'll double-check that it works with camera(s) attached soon, whenever I next have an NDI camera available.

rajkundu commented 1 year ago

I made a new version of ParallelProbe which is more streamlined and I think a bit easier to follow, at least for a beginner like me:

ndicapi* ParallelProbe(bool checkDSR, int maxPortNum)
{
  std::vector<std::future<std::string>> probeTasks;
  for (int i = 0; i < maxPortNum; i++)
  {
    // get serial port #i's full name (e.g., "COM*" on Windows, "/dev/cu.usbserial-******" on macOS)
    const char* portName = ndiSerialDeviceName(i);
    if (portName == nullptr) // port #i doesn't exist
    {
      continue;
    }
    // check if device at port #i is an NDI camera; return its full name if so, otherwise return an empty string
    probeTasks.push_back(std::move(std::async(std::launch::async, [portName, checkDSR]()
      {
        std::string portNameStr(portName); // copy portName to this task's thread
        return ndiSerialProbe(portNameStr.c_str(), checkDSR) == NDI_OKAY ? portNameStr : std::string();
      }
    )));
  }
  // wait for each task to finish; return first (i.e. lowest port # due to above loop) NDI device found
  for (auto& probeTask : probeTasks)
  {
    std::string portName = probeTask.get();
    if (!portName.empty())
    {
      return ndiOpenSerial(portName.c_str());
    }
  }
  return nullptr;
}

I have tested it on macOS, and it works well. I will try to test on Windows next.