opencv / opencv

Open Source Computer Vision Library
https://opencv.org
Apache License 2.0
75.95k stars 55.62k forks source link

fixing cap_pvpapi interface #3322

Closed StevenPuttemans closed 9 years ago

StevenPuttemans commented 9 years ago

This PR tries to fix the following 3 reported issues

A complete remake of the PvAPI API interface solving bugs but also

StevenPuttemans commented 9 years ago

Okay for the person that will look into/review this, please take a look here: https://github.com/StevenPuttemans/opencv_tryout_code/blob/master/camera_interfacing/AVT_manta_camera.cpp

This is a working interface with an AVT Manta Camera. However, if I look at the OpenCV implementation I find many weird constructions that assure the building will never work. Check my remark at http://code.opencv.org/issues/3947

I think we should use this PR to fix as much as possible

StevenPuttemans commented 9 years ago

@asmorkalov I took some time to debug some stuff, basically this capture implementation is completely wrong. It has hardcoded maximum resolution sizes which is just completely wrong and gives very bad results on all camera's that were not used by the original programmer.

I will see if I can make it way more universal and update this PR to include all neccesary fixes.

vpisarev commented 9 years ago

:+1:

StevenPuttemans commented 9 years ago

New discussion, the suggested approach of mono8 being monochrome false and mono16 being monochrome true is incorrect. I have a monochromo manta camera, which returns mono8 as type. I know the data is stored as a bayer pattern for the proscilla series, so afaik the true and false should be switched right?

StevenPuttemans commented 9 years ago

Second discussion: I managed to completely reconfigure the initialization of the camera interface to also support the Manta series of AVT through this API. However, we should have an option to indicate in a VideoCapture if it is a manta or a proscilla series. This is important when configuring the camera settings, Proscilla has more settings then manta.

Suggestion, doing something like

VideoCapture(0 | PVAPI_PROSCILLA) VideoCapture(0 | PVAPI_MANTA)

Could be enough, but I don't really have an idea on how to approach this. @vpisarev @asmorkalov could you guys share some ideas on this?

StevenPuttemans commented 9 years ago

@ all readers, do check the remarks I left. I keep updating the code so they show as outdated diffs but the remarks still stand

StevenPuttemans commented 9 years ago

First big update

I hope someone with a Proscilla camera can find the time to see if it still works for them.

ShaiV commented 9 years ago

Unlike Manta, Prosilica does not generate a device name that includes the string "Prosilica". Therefore, the code starting at line 209 with the definition of Boolean variable PVAPI_PROSCILLA should be changed. My suggestion is to have the default at Prosilica and in case the name contains Manta, change the interface to Manta. This is the suggested code change:

        // Define if we have a PROSCILLA or MANTA GigE camera series
        bool PVAPI_MANTA = false;
        strtok(camInfo.DisplayName, "_");
        if (strcmp(camInfo.DisplayName, "Manta") == 0){
            PVAPI_MANTA = true;
        }

Now whenever you see PVAPI_PROSILCA, change it to !PVAPI_MANTA.

Here's the complete listing that deals with this:

        // Define if we have a PROSCILLA or MANTA GigE camera series
        bool PVAPI_MANTA = false;
        strtok(camInfo.DisplayName, "_");
        if (strcmp(camInfo.DisplayName, "Manta") == 0){
            PVAPI_MANTA = true;
        }
        tPvUint32 frameWidth, frameHeight, frameSize;
        char pixelFormat[256];
        PvAttrUint32Get(Camera.Handle, "TotalBytesPerFrame", &frameSize);
        PvAttrUint32Get(Camera.Handle, "Width", &frameWidth);
        PvAttrUint32Get(Camera.Handle, "Height", &frameHeight);
        PvAttrEnumGet(Camera.Handle, "PixelFormat", pixelFormat,256,NULL);
        if(!PVAPI_MANTA){
            // Retrieve the maximum possible packet size from the camera
            // Then adjust the packet size to that value
            unsigned long maxSize;
            PvAttrUint32Get(Camera.Handle,"PacketSize",&maxSize);
            if(PvCaptureAdjustPacketSize(Camera.Handle,maxSize)!=ePvErrSuccess){
                return false;
            }
        }
        if(PVAPI_MANTA){
            // Explicitly set the width and height correctly since camera can remember configurations
            PvAttrUint32Set(Camera.Handle, "RegionX", 0 );
            PvAttrUint32Set(Camera.Handle, "RegionY", 0 );
            PvAttrUint32Set(Camera.Handle, "Width", frameWidth);
            PvAttrUint32Set(Camera.Handle, "Height", frameHeight);
        }
        // Start the camera
        PvCaptureStart(Camera.Handle);
        if(!PVAPI_MANTA){
            // Set the camera explicitly to capture data frames continuously
            if(PvAttrEnumSet(Camera.Handle, "AcquisitionMode", "Continuous")!= ePvErrSuccess)
            {
                fprintf(stderr,"Could not set Prosilica/Manta Acquisition Mode\n");
                return false;
            }
        }
        if(PvCommandRun(Camera.Handle, "AcquisitionStart")!= ePvErrSuccess)
        {
            fprintf(stderr,"Could not start Prosilica/Manta acquisition\n");
            return false;
        }
        if(PvAttrEnumSet(Camera.Handle, "FrameStartTriggerMode", "Freerun")!= ePvErrSuccess)
        {
            fprintf(stderr,"Error setting Prosilica/Manta trigger to \"Freerun\"");
            return false;
        }
        if (!PVAPI_MANTA){
            // Settings depending on the pixelformat
            if (strcmp(pixelFormat, "Mono8")==0) {
                monocrome = true;
                grayframe = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_8U, 1);
                grayframe->widthStep = (int)frameWidth;
                frame = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_8U, 3);
                frame->widthStep = (int)frameWidth*3;
                Camera.Frame.ImageBufferSize = frameSize;
                Camera.Frame.ImageBuffer = grayframe->imageData;
            }
            else if (strcmp(pixelFormat, "Mono16")==0) {
                monocrome = true;
                grayframe = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_16U, 1);
                grayframe->widthStep = (int)frameWidth;
                frame = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_16U, 3);
                frame->widthStep = (int)frameWidth*3;
                Camera.Frame.ImageBufferSize = frameSize;
                Camera.Frame.ImageBuffer = grayframe->imageData;
            }
            else if (strcmp(pixelFormat, "Bgr24")==0) {
                monocrome = false;
                frame = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_8U, 3);
                frame->widthStep = (int)frameWidth*3;
                Camera.Frame.ImageBufferSize = frameSize;
                Camera.Frame.ImageBuffer = frame->imageData;
            }
            else
                return false;
        }
        if (PVAPI_MANTA){
            monocrome = true;
            grayframe = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_8U, 1);
            grayframe->widthStep = (int)frameWidth;
            Camera.Frame.ImageBufferSize = frameSize;
            Camera.Frame.ImageBuffer = grayframe->imageData;
        }
        return true;
ShaiV commented 9 years ago

The change that polls the number of cameras is not working for me. This is due to the wait implementation inside the while loop (this might be different on Linux and Windows). The suggested code change for Windows is this (notice the addition of Sleep(10)):

    if (!PvInitialize()) {
        // Wait until the initialisation actually succeeded in opening up a camera
        // Ensure multiple cameras can be opened if preferred by grabbing the current amount before the initialisation
        while((PvCameraCount() != (numCameras+1)) && (initializeTimeOut--))
        {
            cvWaitKey(10);
            Sleep(10);
        }
        if (initializeTimeOut == 0){
            fprintf(stderr,"ERROR: camera intialisation timeout [5000ms].\n");
        }
        numCameras=PvCameraList(cameraList, MAX_CAMERAS, NULL);
    }

I'm not sure why you switch between PvCamerList() function and PvCameraCount(). Wouldn't it be best to use just one? They seem to be interchangeble. I haven't tried this on Linux yet.

ShaiV commented 9 years ago

There is also an issue when you have two cameras with two different interfaces installed. For example, I have a webcam which is a USB device and a Prosilica. The prosilica is device 1; the USB is device 0.

This causes issues with indexing since the code assumes that the camera with index 1 is the second PVAPI camera (counting starts at zero). Which is wrong-- it is actually the first camera. There are several places we have to touch to fix this.

  1. The sanity check in line 166:
    if (numCameras <= 0 || index >= (int)numCameras){

    Here the check for index>=(int) numCameras should be removed. Since again, counting does not start at zero. And camera with index 2 might be the first camera.

  2. The same problem also appears in 171. The index value may be 1 as requested by the user whereas the camera list might only have one element.

The fix is not so simple. It should also involve changing how the index variable is treated.

ShaiV commented 9 years ago

In line 265 under "mono8", monocrome flag is set to false which is wrong. It should be set to true. Also, all "mono16" should be set to monocrome, not color. Only Bgr24 should be set to color.

StevenPuttemans commented 9 years ago

Thanks for all your remarks! Will adapt where possible :)

Some remarks / discussion

  1. Adapted to the !PVAPI_MANTA approach.
  2. After reading again both manta and proscilla can use the same loop for type definition, so combined again. It can be that my further problems are due to the fact that the color image isn't created in the manta only loop --> this i need to check at work on monday.
  3. I need to use PvCameraList once to initialize camera information for further grabbing the UID of the cam. The others are all replaced by PvCameraCount to be uniform coding.
  4. Discovered that Manta only needs the Freerun option, the acquisition mode and acquisition start are parameters that don't influence the camera working process so put them to prossilla only.
  5. adapted the monocrome true and false statements
StevenPuttemans commented 9 years ago

As to the index, I suggest we get the code running for 1 proscilla or 1 manta first; simply disconnect the others for a moment. Lets then make a new PR and bug for the multiple camera handling and indexing :)

StevenPuttemans commented 9 years ago

There was a trailing whitespace error whch I resolved. As to Shai, can you explain what errors are generated by cvWaitKey(0)? Because this works perfectly on my system.

StevenPuttemans commented 9 years ago

O forget my last remark; found the reason why this fails. I just recalled that cvWaitKey ONLY works if there is an active window available. Since this is not the case at initialization, the wait never happens.

I will change it into a sleep command!

ShaiV commented 9 years ago

Steven, cvWaitKey() does not generate any errors. However, on my system it does not actually wait. Hence, the code that supposedly waits for 5 seconds exits very fast without any timeout and the result is that the camera is not identified properly (the code does not wait enough time for it to initalize). If you add Sleep(10) after cvWaitKey(10) then an actual wait is performed and things work OK.

StevenPuttemans commented 9 years ago

Yeah just discovered the same thing, since I was debugging and I moved instruction by instruction it worked. I am guessing that this would solve my problem at work also, but thats for monday.

BTW you know the reason of creating a 3 channel frame image in the monochrome cases? Seems overkill...

ShaiV commented 9 years ago

I would also suggest changing the strcmp() function calls to stricmp() function calls when checking for pixelformat; it is a little more robust. This is for "mono8", "mono16" and "Bgr24" strings.

Here's something that's been bothering me. In case of "mono8" and "mono16", there is no need to generate a color frame. I've checked this and it seems to work OK with "mono8":

        if (!PVAPI_MANTA){
            // Settings depending on the pixelformat
            if (stricmp(pixelFormat, "Mono8")==0) {
                monocrome = true;
                grayframe = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_8U, 1);
                grayframe->widthStep = (int)frameWidth;
                /*
                frame = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_8U, 3);
                frame->widthStep = (int)frameWidth*3;
                */
                Camera.Frame.ImageBufferSize = frameSize;
                Camera.Frame.ImageBuffer = grayframe->imageData;
            }
            else if (stricmp(pixelFormat, "Mono16")==0) {
                monocrome = true;
                grayframe = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_16U, 1);
                grayframe->widthStep = (int)frameWidth;
                /*
                frame = cvCreateImage(cvSize((int)frameWidth, (int)frameHeight), IPL_DEPTH_16U, 3);
                frame->widthStep = (int)frameWidth*3;
                */
                Camera.Frame.ImageBufferSize = frameSize;
                Camera.Frame.ImageBuffer = grayframe->imageData;
            }

Notice the commenting of the lines associated with the frame variable (only grayframe variable is used). This will be a little more memory efficient and slightly faster. But requires some more testing (it works well on my system).

ShaiV commented 9 years ago

That's exactly what I wrote-- no need for a 3 color frame.

StevenPuttemans commented 9 years ago

Will change! And again, Linux Manta test on monday :)

ShaiV commented 9 years ago

This is something I wanted to discuss. First of all, I don't like the use of the PVAPI_MANTA variable. I read through the code and came to a conclusion that the reason for this lies in the topic explained below: initilization of camera parameters.

In my view, changing camera's parameters upon initialization, unless absolutely necessary, should not be done. The necessary parameters are: Setting the AcquisitionStart, setting the AcquisitionMode and setting the FrameStartTriggerMode. All other parameters should not be set on PvCameraOpen().

The reason is that users set the values with the GigEViewer application which comes with the camera's SDK. For example, say I want a slower bit-rate so as not to clog my bandwidth, then adjusting the maximum packet size automatically in PvCameraOpen() will not allow me to do that. The same goes for ROI. For example, the lines of code:

        if(!PVAPI_MANTA)
        {
            // Retrieve the maximum possible packet size from the camera
            // Then adjust the packet size to that value
            unsigned long maxSize;
            PvAttrUint32Get(Camera.Handle,"PacketSize",&maxSize);
            if(PvCaptureAdjustPacketSize(Camera.Handle,maxSize)!=ePvErrSuccess){
                return false;
            }
        }

Should be removed. It's up to the user that configured the camera to set this value. The code in PvCameraOpen() should only open the camera and set it to read frames. If the user wishes to change camera parameters he should do so explicitly after the camera is opened. This can be done by adding an overloaded function setPropery() to the camera that accepts two flavors: setProperty(string, string) and setProperty(string, int). This will allow the user to change all sorts of camera parameters.

More notes: The code

        if(!PVAPI_MANTA)
        {
            // Set the camera explicitly to capture data frames continuously
            if(PvAttrEnumSet(Camera.Handle, "AcquisitionMode", "Continuous")!= ePvErrSuccess)
            {
                fprintf(stderr,"Could not set Prosilica/Manta Acquisition Mode\n");
                return false;
            }
        }

Why is there an if(!PVAPI_MANTA) here? Shouldn't this be for all cameras?

Once you remove all these initializations, there should be no reason to have the PVAPI_MANTA flag. You could merge everything to one mainline. The default acquisition mode should be set to mono8 so as to support MANTA.

Thanks,

ShaiV commented 9 years ago

Two issue: a bug with the initialization process and the index issue in case several cameras of different protocols are installed on the system with PVAPI cameras as well.

The problem with the initialization code is that first we issue a call to:

unsigned int numCameras = PvCameraList(cameraList, MAX_CAMERAS, NULL);

and only then we call PvInitialize(). The problem is that PvCameraList() populates the array variable cameraList with all the visible cameras. However, since PvInitialize() is not called before PvCameraList(), the list is just random values without any cameras because there was no camera initialization. Therefore, after a call to PvInitialize(), one must also call the PvCameraList() to properly populate the cameraList array (see below for a fix).

The second issue is with the index variable that is passed as a parameter to the function. The problem is that this index is just a random number that has no meaning, since the cameras are identified with the Unique ID variable. To be able to open the camera, all that is needed is to set the Unique ID to the last value in the camerList array.

This approach will ensure that even if you have a different protocol camera (for example, USB and PvAPI), the index will be properly set. The problem is if you have two PVAPI cameras. I'm not sure how this will work-- it needs to be tested.

Here's the suggested code change that deals with both the index issue and the initialization process.

    int initializeTimeOut = 500;
    unsigned int numCameras = PvCameraCount();
    if(PvInitialize()==ePvErrSuccess)
    {
        // Wait until the initialisation actually succeeded in opening up a camera
        // Ensure multiple cameras can be opened if preferred by grabbing the current amount before the initialisation
        while((PvCameraCount()!=(numCameras+1)) && (initializeTimeOut--))
            Sleep(10);
        if(!initializeTimeOut)
        {
            fprintf(stderr,"ERROR: camera initialization timeout [5000ms].\n");
            return false;
        }
        numCameras = PvCameraList(cameraList, MAX_CAMERAS, NULL);
    }
    else
    {
        fprintf(stderr, "ERROR: Some required system resources were not available, PvInitialize().\n");
        return false;
    }
    // no cameras found
    if(!numCameras)
    {
        fprintf(stderr, "ERROR: No cameras found.\n");
        return false;
    }

    // since we've just initialized the camera with PvInitialize(), it is the last camera on our list
    // notice how PvInitialize() does not care about the index and so we shouldn't as well-- it has its own
    // unique ID, UID.
    Camera.UID = cameraList[numCameras-1].UniqueId;
    if(PvCameraIpSettingsGet(Camera.UID,&ipSettings)==ePvErrNotFound)
    {
        fprintf(stderr, "The specified camera UID %lu could not be found, PvCameraIpSettingsGet().\n", Camera.UID);
        return false;
    }

    if(PvCameraInfo(Camera.UID,&camInfo)==ePvErrNotFound)
    {
        fprintf(stderr, "The specified camera UID %lu could not be found, PvCameraInfo().\n", Camera.UID);
        return false;
    }
    else
    {
        /*
        struct in_addr addr;
        addr.s_addr = ipSettings.CurrentIpAddress;
        printf("Current address:\t%s\n",inet_ntoa(addr));
        addr.s_addr = ipSettings.CurrentIpSubnet;
        printf("Current subnet:\t\t%s\n",inet_ntoa(addr));
        addr.s_addr = ipSettings.CurrentIpGateway;
        printf("Current gateway:\t%s\n",inet_ntoa(addr));
        */
    }
StevenPuttemans commented 9 years ago

Lets fire up the discussion

  1. I agree that initialization is important not to include specific configuration details for the camera. However, Manta doesn't have a Acquisition mode option, since that camera can only grab continously. Therefore that configuration pushes an error and thus it stops initializing. This is the only reason why the PVAPI_MANTA parameter is actually needed.
  2. I agree that the specific format setting is something that needs to be done by the user. However I do not agree that this should be done with the gigEviewer. Instead I think we should expand the getters and setter functions to push more functionality and warn there if a camera doesn't support the option.
  3. The overload functions is indeed a good approach
  4. As to why that extra !PVAPI_MANTA is there for the AcquisitionMode, simply because that function is NOT supported by Manta camera
  5. I will deal with your adaptations tonight

As a small remark. If you plan to discuss code further, place this as remarks inside the code at the files changed tab. This will keep the discussion a bit more clear. Thanks again for your help!

ShaiV commented 9 years ago

Thanks Steven, I didn't know about the Manta specific parameters. If that is the case, then do please leave the Manta code as is.

I've tried implementing my suggestion with an overloaded function but the details are too deep and will cause an overhaul of the entire VideoCapture class so for now I think I'll leave it as is. It's good enough for now. Myabe when I have some more time on my hands. While we're at it, the VideoCapture(index) mechanism should be changed to accomodate for this.

Now to tackle a few more bugs/issues. They all creep up when you start hooking up more than one camera. The first issue is if you call PvInitialize() more than once, it will fail (the DLL is already open). This is probably why the original author did not check the PvInitialize() return value, and is indeed a required change.

As suspected, although my initial fix was able to allow for different interface cameras (USB and PVAPI) to work well, it did not handle several PVAPI cameras. This is because PvCameraList() lists all the cameras and not just one after the other.

I therefore implemented a different mechanism. The code keeps trying to open cameras in the cameraList until all the list is exhausted. This way successive calls to VideoCapture() will go through all the PVAPI cameras and you will be able to access them all. See the code that uses the variable findNewCamera for details.

I've made several changes to the function bool CvCaptureCAM_PvAPI::open(int index). I've tried to leave remakrs whenever applicable. By the way, I did a dummy index++ call just so the warning message of an unsued variable will not be shown; if you know of a better way please change this.

First, the change to PvInitialize():

    tPvCameraInfo cameraList[MAX_CAMERAS];
    tPvCameraInfo  camInfo;
    tPvIpSettings ipSettings;
    index++;  // to avoid a warning, we don't really use index
    // Initialization parameters [500 x 10 ms = 5000 ms timeout]
    int initializeTimeOut = 500;
    unsigned int numCameras = PvCameraCount();
    // disregard any errors, since this might be called several times and only needs to be 
    // called once or will return an error
    PvInitialize(); 
    // this if sentence is not needed due to PvInitialize() being called without reading ther result
    //if(PvInitialize()==ePvErrSuccess)
    {
        // Wait until the initialisation actually succeeded in opening up a camera
        // Ensure multiple cameras can be opened if preferred by grabbing the current amount before the initialisation
        while((PvCameraCount()!=(numCameras+1)) && (initializeTimeOut--))
            Sleep(10);
        if(!initializeTimeOut)
        {
            fprintf(stderr,"ERROR: camera initialization timeout [5000ms].\n");
            return false;
        }
        numCameras = PvCameraList(cameraList, MAX_CAMERAS, NULL);
    }
    // the else branch is no longer needed
    /*
    else
    {
        fprintf(stderr, "ERROR: Some required system resources were not available, PvInitialize().\n");
        return false;
    }
    */
    // no cameras found
    if(!numCameras)
    {
        fprintf(stderr, "ERROR: No cameras found.\n");
        return false;
    }

Now the change to read the next available PVAPI camera:

// try opening the cameras in the list, one-by-one until a camera that is not used is found
unsigned int findNewCamera;
for(findNewCamera=0; findNewCamera<numCameras; findNewCamera++)
{
    Camera.UID = cameraList[findNewCamera].UniqueId;
    if(PvCameraOpen(Camera.UID, ePvAccessMaster, &(Camera.Handle))==ePvErrSuccess)
        break;
}
if(findNewCamera == numCameras)
{
    fprintf(stderr, "Could not find a new camera to connect to.\n");
    return false;
}

if(PvCameraIpSettingsGet(Camera.UID, &ipSettings)==ePvErrNotFound)
{
    fprintf(stderr, "The specified camera UID %lu could not be found, PvCameraIpSettingsGet().\n", Camera.UID);
    return false;
}

if(PvCameraInfo(Camera.UID, &camInfo)==ePvErrNotFound)
{
    fprintf(stderr, "The specified camera UID %lu could not be found, PvCameraInfo().\n", Camera.UID);
    return false;
}
else
{
    /*
    struct in_addr addr;
    addr.s_addr = ipSettings.CurrentIpAddress;
    printf("Current address:\t%s\n",inet_ntoa(addr));
    addr.s_addr = ipSettings.CurrentIpSubnet;
    printf("Current subnet:\t\t%s\n",inet_ntoa(addr));
    addr.s_addr = ipSettings.CurrentIpGateway;
    printf("Current gateway:\t%s\n",inet_ntoa(addr));
    */
}

// this if sentence is no longer needed since the camera was opened with the findNewCamera code
//if (PvCameraOpen(Camera.UID, ePvAccessMaster, &(Camera.Handle))==ePvErrSuccess)
{
    // If your camera supports multiple pixel formats and you explicitly want to set the BGR24 format,

Thanks,

PS, I'm not that familiar with Git and don't know how to add remarks inside the code; I'll look it up for the next time.

StevenPuttemans commented 9 years ago

Thanks for the updates. Wil fix prolly tomorrow. To busy now.

StevenPuttemans commented 9 years ago

Everything applied. I will keep an eye on the output of the buildbot and let you know if any errors pop up :) Also squashed all commits into a single one.

ShaiV commented 9 years ago

I'm afraid another change is needed. The approach with checking that the number of cameras has increased by 1 in the while loop is inaccurate as the function PvCameraCount() will return all the cameras at once. A more accurate approach (and as suggsested by AVT manual) is to check that the value is non-zero. If it's non-zero it means at least one camera is detected. And if one camera is detected, all cameras should be detected as well; so in reality, the value might increase by more than 1.

This issue might pop up in several situations: (1) several cameras connected at once; (2) deleting the VideoCapture object and generating new one; this is something that needs to be furhter tested and probably added as a test case.

Therefore, another minor change should be applied as follows, in function bool CvCaptureCAM_PvAPI::open(int index):

    index++;  // to avoid a warning, we don't really use index
    // Initialization parameters [500 x 10 ms = 5000 ms timeout]
    int initializeTimeOut = 500;
    // disregard any errors, since this might be called several times and only needs to be 
    // called once or will return an error
    PvInitialize(); 
    // Wait until the initialisation actually succeeded in opening up a camera
    // Ensure multiple cameras can be opened if preferred by grabbing the current amount before the initialisation
    while((!PvCameraCount()) && (initializeTimeOut--))
        Sleep(10);
    if(!initializeTimeOut)
    {
        fprintf(stderr,"ERROR: camera initialization timeout [5000ms].\n");
        return false;
    }

    unsigned int numCameras = PvCameraList(cameraList, MAX_CAMERAS, NULL);
    // no cameras found
    if(!numCameras)
    {
        fprintf(stderr, "ERROR: No cameras found.\n");
        return false;
    }

From here on it should be as before.

Thanks,

StevenPuttemans commented 9 years ago

Small remark - testing it on my machine now, but stricmp is not recognized here. So I sticked to strcmp.

StevenPuttemans commented 9 years ago

Okay a wrapup. After using the following code snippet on an openCV build only with PvAPI camera support, it seems the following sample doesn't work out.

#include <iostream>
#include "opencv2/opencv.hpp"

using namespace std;
using namespace cv;

int main()
{
    VideoCapture camera(0);
    Mat frame;

    while(true){
        camera >> frame;
        imshow("test", frame);
    }

    cout << "Hello world!" << endl;
    return 0;
}

It returns an empty frame from the camera. Will dig into it asap!

StevenPuttemans commented 9 years ago

The following line of code still fails for my Manta cam, the code above seems to work. Will make a printout of all parameters to check the problem.

if(PvCameraInfo(Camera.UID,&camInfo)==ePvErrNotFound)
    {
         fprintf(stderr, "The specified camera UID %lu could not be found, PvCameraInfo().\n", Camera.UID);
         return false;
    }
StevenPuttemans commented 9 years ago

Update: seems that the capturing works now. After identifying some typos and wrongdoings, I can now move on to the grabbing of the frame. It seems that there is still a problem.

StevenPuttemans commented 9 years ago

Removed the manta checks, seems this was due to other hickups that the call didn't work. This is also way cleaner.

StevenPuttemans commented 9 years ago

It works! Manta camera is opening without any problems! HURAY for us and thanks in advance for the help.

@asmorkalov or @vpisarev this is ready to get checked. @ShaiV any remarks?

StevenPuttemans commented 9 years ago

And this was the test code

#include <iostream>
#include "opencv2/opencv.hpp"

using namespace std;
using namespace cv;

int main()
{
    VideoCapture camera(0);
    Mat frame;

    while(true){
        camera >> frame;
        imshow("test", frame);
        int key = waitKey(10);
        if(key == 27){
            break;
        }
    }
    return 0;
}
StevenPuttemans commented 9 years ago

Also @ShaiV it would be nice if you can test the end result and see if it still works properly on your end.

StevenPuttemans commented 9 years ago

My students are already using this fix extensively. Lets merge it :)

ShaiV commented 9 years ago

I'll have some time to look at it this weekend. All issues resolved? Shai

Shai Vaingast Author of Beginning Python Visualization 2nd Edition http://www.apress.com/9781484200537 בתאריך 13 באוק 2014 20:18, "Steven Puttemans" notifications@github.com כתב:

My students are already using this fix extensively. Lets merge it :)

— Reply to this email directly or view it on GitHub https://github.com/Itseez/opencv/pull/3322#issuecomment-58924887.

StevenPuttemans commented 9 years ago

Yep, proof can be seen here :

IMAGE

I got it to work even with multiple cams in our lab. So I guess it will work at your end too. Only thing I would like to add is a place where we can make people know that they have to specificy the cap interface in the VideoCapture.

For example, if you have both Gstreamer and PvAPI configured in Linux, then Gstreamer will try to pull first because it recongnizes that there is a media device linked to the network card. However it fails when setting up the gstreamer pipeline.

This can be countered by explicitly telling the videocap to use the PvAPI.

VideoCapture input(0 + CV_CAP_PVAPI);

However this is documented nowhere at all. I am thinking of adding a capture_avt_cameras.cpp sample. The picture doesnt contain this fix since i builded OpenCV exclusively with PvAPI as capture interface there.

ShaiV commented 9 years ago

Edit: I saw that you removed the Manta special case. Perfect! Steven I didn't find any different initialization to Manta and Prosilica. Were you able to merge them into one? If so that's great. Leaves a very clean code. (I still haven't tested the code)

On Mon, Oct 13, 2014 at 9:31 PM, Steven Puttemans notifications@github.com wrote:

Yep, proof can be seen here :

[IMG]http://i62.tinypic.com/707ukm.png[/IMG]

I got it to work even with multiple cams in our lab. So I guess it will work at your end too. Only thing I would like to add is a place where we can make people know that they have to specificy the cap interface in the VideoCapture.

For example, if you have both Gstreamer and PvAPI configured in Linux, then Gstreamer will try to pull first because it recongnizes that there is a media device linked to the network card. However it fails when setting up the gstreamer pipeline.

This can be countered by explicitly telling the videocap to use the PvAPI.

VideoCapture input(0 + CV_CAP_PVAPI);

However this is documented nowhere at all. I am thinking of adding a capture_avt_cameras.cpp sample.

— Reply to this email directly or view it on GitHub https://github.com/Itseez/opencv/pull/3322#issuecomment-58934675.

Shai Vaingast Author of Beginning Python Visualization, 2nd Edition http://www.apress.com/9781484200537

StevenPuttemans commented 9 years ago

Edit: I saw that you removed the Manta special case. Perfect! Steven I didn't find any different initialization to Manta and Prosilica.

Well it seems that due to a faulty indexing beforehand, the error only returned when reaching those checks. Therefore I assumed the mode selection did not work on Manta type camera. However after looking into the manta manual it seemed that Manta provided the same functionality. So right after I got it all to work on your hints, I decided to remove the checks for Manta, and it seems that it worked perfectly fine.

As to the initialization of the window. I did that explicitly since in my lab people tend to use the camera, leave different settings on it, but then do not reinitialize it to standard values. Since these AVT cameras can remember configurations, this can sometimes be a pain in the ass. But again, this should be covered with setters and getters, not with the initialization.

StevenPuttemans commented 9 years ago

Added a small sample. This can later be used to add examples of how to address the properties. Want to keep this PR as basic as possible before we move on to other fixes in this API.

ShaiV commented 9 years ago

Works well with two Prosilica cameras in color and in monochrome modes. Some minor cosmetic comments on the code to follow.

ShaiV commented 9 years ago

Like I said, works very well and should be committed.

StevenPuttemans commented 9 years ago

All minor fixes applied and fixed the warnings and build errors. @asmorkalov could you take the time to review it now?

StevenPuttemans commented 9 years ago

PING! :)

asmorkalov commented 9 years ago

It looks ok for me. Please look at my comments and I can merge it after fixes.

StevenPuttemans commented 9 years ago

Updated!

StevenPuttemans commented 9 years ago

@asmorkalov fixed it, could be best to merge it now?

asmorkalov commented 9 years ago

:+1:

StevenPuttemans commented 9 years ago

Tss I broke it :D Let me restore :)