Changed SpinnakerCamera StreamBufferCountMode

[jacksulli]

Previously, the buffer count mode for the Spinnaker Camera in blacs_workers.py was set to Auto. This Auto feature has been depreciated by FLIR so I changed it to be manual in the same format as the continuous mode. In the case where bufferCount == 1, I set it to 3 because the Spinnaker SDK mentioned that 3 was the minimum buffer count.

[dihm]

Thank you for the bug fix @jacksulli, it is much appreciated! I have only have three questions:

1. Do you know what version of the SDK deprecated the Auto feature?
2. Is the change you've made backwards compatible with older SDKs? If it isn't, we'll want to put in checks to capture the SDK version and select the correct mode based on that.
3. (edit) Why did you change the stream mode to take newest images first instead of oldest?

[jacksulli]

Thank you for the bug fix @jacksulli, it is much appreciated! I have only have three questions:

1. Do you know what version of the SDK deprecated the Auto feature?
2. Is the change you've made backwards compatible with older SDKs? If it isn't, we'll want to put in checks to capture the SDK version and select the correct mode based on that.
3. (edit) Why did you change the stream mode to take newest images first instead of oldest?

No problem @dihm! Version 3.0.0.118 depreciated the feature, which I found here: https://www.flir.com/support-center/iis/machine-vision/knowledge-base/spinnaker-sdk-release-notes/

I think that it should still be backwards compatible, since I just used the same format for the buffer mode that the continuous acquisition mode used. As long as that still works with previous versions, I think these changes should as well.

The only reason I switched it to newest first was because I was just trying to copy the format of the continuous mode, so that might not be right. I probably should have looked into that a bit more!

[dihm]

@jacksulli Sorry for continuing to be slow here. I don't have an easy way to test this here and things keep piling up.

The only reason I switched it to newest first was because I was just trying to copy the format of the continuous mode, so that might not be right. I probably should have looked into that a bit more!

Yeah, this is an explicit choice the camera drivers make (when available). When streaming in continuous mode, you don't really care about frame order or dropping frames, so you always choose newest first in case the computer gets slow relative to the camera. When in buffered mode, dropped frames and order are critical considerations, so we always pull the last frame first to ensure it gets read out before the buffer overflows. We should continue to respect that distinction.

I also think we could probably modify this if-else branch since buffercount of 1 isn't allowed anymore anyway. I'll suggest some changes directly in the code itself. If you could make a few modifications, I'll go ahead and merge.

[dihm]

It is probably also sporting to add a comment in this function mentioning the feature that has been deprecated and which version did it. That way if this breaks something for someone and they start looking, they can find the culprit quickly.

[jacksulli]

@jacksulli Sorry for continuing to be slow here. I don't have an easy way to test this here and things keep piling up.

The only reason I switched it to newest first was because I was just trying to copy the format of the continuous mode, so that might not be right. I probably should have looked into that a bit more!

Yeah, this is an explicit choice the camera drivers make (when available). When streaming in continuous mode, you don't really care about frame order or dropping frames, so you always choose newest first in case the computer gets slow relative to the camera. When in buffered mode, dropped frames and order are critical considerations, so we always pull the last frame first to ensure it gets read out before the buffer overflows. We should continue to respect that distinction.

I also think we could probably modify this if-else branch since buffercount of 1 isn't allowed anymore anyway. I'll suggest some changes directly in the code itself. If you could make a few modifications, I'll go ahead and merge.

Sounds good, I've made the changes you suggested. I also looked again in the Spinnaker documentation but I also couldn't find where it said the buffer count minimum should be 3 anymore, so I'm not sure where I found that. I tested it a little bit and taking a single snap with both the UI and through RunManager seemed to work fine with the buffer count set to 1.

[dihm]

LGTM. Thanks for the fix!