john-judge / PhotoZ_upgrades

CCD controller, data acquisition, and stimulus administration software for Dr. Meyer Jackson's hVOS voltage imaging technique. This repository tracks the transition from PhotoZ OG to NI-DAQmx and the "Little Dave" DM2K Camera
MIT License
1 stars 0 forks source link

Image reassembly: CDS subtraction and deinterleaving #8

Closed john-judge closed 3 years ago

john-judge commented 3 years ago

There's no direct documentation for the Little Dave camera. Technical information must be aggregated from Chun's TurboSM code, general documentation for camera hardware, direct observation, and correspondence with Chun.

DV-1K-no-cds Camera is behaving as Chun would expect. subtractCDS() is a function in SM_take_tb-Chun.cpp which gets called when the cdsFlag is set.

Making sense of the algorithm used for this Correlated double sampling (CDS) noise reduction technique, it seems like this code is the heart of the method ( "The dark columns are the reset values for the next frame - image part of the same channel (16 channels for 2K)." ) --

 for (row = 0; row < rows; ++row) {
     for (col = cols; col; --col)
            *new_data++ = CDS_add + *old_data++ - *reset_data++;
      new_data += cols;
      reset_data += cols;
      old_data += cols;
}
john-judge commented 3 years ago

Added sm.cpp to documentation folder.

sm.cpp bridges the gap between sm_config.dat (in the documentation folder of PhotoZ) and SM_take_tb-Chun.cpp.

Tracing through sm.cpp, the ROTATE flag and the Bad_pix (used in Chun's frameRepair function) are turned off for Lil Dave. The TwoK flag is turned on; the QUAD flag is turned off.

mapping the SM_config.dat values to the arguments used here.

// arguments: stripes = image_stripes, layers = image_layers, factor = superFrame
// SM_take_tb(pdv_chan_num,     file_name,          , ndr_lib[curConfig],   curNFrames,     
// SM_take_tb(int numChannels,  char *file_name,    ,   int NDR_flag,       int images,                 

// cds_lib[curConfig]       =>  int cdsF = 1
// layers_lib[curConfig],   =>  int layers = 1
// stripes_lib[curConfig],  =>  int stripes = 4
// rotate_lib[curConfig],   =>  rotateFlag = 0
// bad_pix_lib[curConfig],  =>  bad_pix_index = 0
// ndr_lib[curConfig],      =>  NDR_flag = 0
// ccd_lib_bin[curConfig] / 2 is passed to value of h_bin, and varies for each little dave program
// super_frame_lib likely determines superFrame and thus factor?

// frame_interval,          f_size_ratio,       cameraType,         file_frame_w,
// double frame_interval,   int factor,         char *cameraname,   int file_img_w, 

//BNC_ratio,        ExtTriggerFlag,     preTrigger_frames,      dark_f_flag,    QuadFlag,            segment_lib[curConfig]);
//int BNC_ratio,    int ExtTriggerFlag, int preTrigger_frames,  int darkFlag,   int quadFlag,        int segmented)
if (strstr(cameraType, "DM2K")) {
        TwoK_flag = true;
        QuadFlag = FALSE;
        pdv_chan_num = 4;
    }
john-judge commented 3 years ago

Helpful info from Chun clarifying the specs in documentation/quadrant_readout.docx:

If you shine light to the sensor, you will see the bright section is the data, and dark section is the reset for the next frame.

Let’s just focus on 1 quadrant (1 pdv channel). Let’s say you are working on 4000Hz - 512x80 CDSBIN4. Each quadrant you;ld be 256x40. The CDS data would be 512x40 because it always double the width. There 4 camera channels within each pdv channel, 256/4 = 64. So it means each camera channel has 64 pixels per row.

Before deinterleaving, the raw data order for each row comes in like this (d for pixel data, r for pixel reset, which would be used for the next frame):

d1, d64, d128, d192, d2, d65, d129, d192, d3,…d256, r1, r64, r128, r192… r256 (total 512)

My notes: I wasn't aware of the fact that the reason for the interleaving was there are camera channels within each PDV channel. This affects the code because I originally thought the interleaving was supposed to interleave across PDV channels, whereas in reality it only interleaves across camera channels within a quadrant. I didn't know we needed to double the width to accomodate CDS. To accomodate, need to double the memcpy striding:

memcpy(memory + (2*array_diodes*i), image, 2*array_diodes);  

Also in MainControllerAcqui.cpp to make sure that the call from MainController:takeRli to DapController::takeRli allocates enough memory. E.g.

short *memory=new short[bufferSize*475];

Note currently bufferSize is only the size of a single quadrant, and 475 is the number of images needed for taking RLI. Multiply this by 4 to account for the fact that bufferSize is now a quadrant size, not an image size, and then by 2 to account for CDS (for a total factor of 8).

john-judge commented 3 years ago

Putting the data in 1-D, there are clearly 5 rows, meaning that rows are 1024 px (1024 data pixels followed by 1024 reset pixels):

image

john-judge commented 3 years ago

readout-RLI-5

The last piece of the puzzle was the channel readout order. In Chun's code, the channelOrder of the 4 camera channels is { 3, 2, 0, 1 }. I produced the above image with a channel readout order of { 2, 3, 1, 0 }. The reason is that Chun uses this to map from source locations, while I use this to map to destination locations. This became clear to me in the example assertions in UnitTests1/utCamera.cpp

Increasing CDS_add prevented these aberrations where the pixel data should be approximately 0 after reset subtraction. The downside of increasing CDS_add is that there's a risk of saturating at bright values which are real data -- it's better to see aberrations in dark patches than in real data. I don't think CDS_add has any calibration significance, otherwise Chun would probably let this value be configurable rather than hard-coding it.

john-judge commented 3 years ago

Image Size Calculations

The char versus short size also affects the image size calculations. It might be causing the confusion with what you've noticed as seemingly inconsistent image sizes reported by PDV:

Unsigned char values are 1-byte.

Each unsigned short is 2 bytes, which means that when PDV reports:

image size (bytes per image) 10240

the actual array size is 5120 pixels, each pixel represented by a 2-byte short value.

Recommendations for pdv_setsize

Any changes we need to make to image dimensions should be done through the settings and directives in the .cfg files, rather than through pdv_setsize.

I don't believe pdv_setsize is needed based on 1) LilDave not needing superframes and 2) given the way PhotoZ is structured.

Setting the size manually with pdv_setsize would only be needed for one of two reasons: 1) High-speed camera configurations using superframe (Chun) 2) To transition between camera programs without multiple initcam calls. Probably not safe unless we have a very detailed understanding of the PDV API.

For reason #1 -- Chun's recommendation: "In general, once you load the *.cfg through initcam, the correct width and height are loaded." Chun described pdv_setsize only for superframe'd cameras such as "DaVinci". The idea is that the readout needs to be in batches of 10 for the software to keep up with the hardware, and Chun's method for this is to use pdv_setsize to override to superframe image sizes.

For reason #2 -- PhotoZ just uses multiple calls to initcam. No need to transition between image sizes manually.

Just for reference, a summary of how PhotoZ architecture uses multiple initcam calls to transition between camera programs:

When a user selects a new camera setting in PhotoZ, the choice of camera program (0-7) is stored in the DapController global object, dc. Only when the buttons trigger MainController's acqui or takeRli does the dc camera program setting get loaded into the newly created Camera object. The Camera object calls initcam, passing the .cfg filename based on that program setting, which sets the proper image height and width. Only after the Camera object is destroyed may the user select a new program, which will create a new Camera object to call initcam again with the new program setting.

Given that initcam is meant to be a command-line program, I'd be surprised if it weren't ok to call it multiple times. Since PDV's source file initcam.c code commeents mention freeing buffers needed between multiple calls to pdv_initcam, it seems like the PDV programmers took multiple calls into account.

john-judge commented 3 years ago

Documenting technical details for byte alignment issue.

Short version

Little Dave reads out 2-byte pixels. Use a unsigned short* pointer to manipulate the raw image pixel-by-pixel.

Long explanation (for C++ beginners):

Meyer ran into an issue when trying to print out pixel values as follows:

for (int k = 0; k < array_diodes; k++) cout << "DapC line 667  " << k << " " << short (image[k]) << "\n";

The type of an array affects how C++ behaves when you use pointer arithmetic or access the array by index. In the above code, image is of type unsigned char*, so the kth char is found, i.e. we use 1-byte alignments for the access. It doesn't matter that you cast to short just after, because all that does is allow that 1-byte char to be padded with a prefix byte of zeros if needed.

What we really need is for the kth element to be found using 2-byte alignment, i.e find the kth short.

So the fix for the printout should be to 1) declare 'memory' as an unsigned short* and 2) update to this (look at the kth short of the last RLI image):

for (int k = 0; k < array_diodes; k++) cout << "DapC line 667  " << k << " " << memory[array_diodes * (rliPts-1) + k] << "\n";

Similarly, when we are "Arranging Data from Memory to traceData" on line 79-83 of MainControllerAcqui, having the wrong type will cause this to index into half-pieces of shorts, since our indexes are treated as 1-byte aligned rather than 2-byte aligned. This becomes an issue here:

traceData[i][j]=memory[i+j* bufferSize];

So, again, the memory array should remain unsigned short* as it was before, to avoid more alignment issues. Yes, the return type of pdv_wait_image is unsigned char* (because PDV must support some cameras and settings with 1-byte pixels), but memcpy handles the cast implicitly from the unsigned char* image to the unsigned short* memory as long as we tell it the correct number of bytes to copy.

john-judge commented 3 years ago

This is the 300th image read out while we're taking the RLI.

unnamed (2)

allocated size 40960 image size (bytes per image) 40960 buffer size 40960 image height 20 cam height 20 image width 1024 cam width 1024

PDV said that the image size was 40960 bytes. Following Chun's current instructions for a 1024px row, we ended up with a 1024 x 10 px image for this quadrant.

john-judge commented 3 years ago

Multiple PDV channels read out in parallel. Empirically arrived at a reasonable quadrant-specific channel ordering based on results.

New issues (at least with this data from mid-Feb):

But this result still appears to suggest the true quadrant-specific camera channel orders. Also, this appears to suggest that the PDV channels are readout in this order: upper left, lower left, upper right, lower right.

image

Channel orderings used:

int channelOrders[16] = 
{ 2, 3, 1, 0, 
  1, 0, 2, 3,
  2, 3, 1, 0,
  1, 0, 2, 3, };

*Artifacts due to row misalignment issue? There appears to have been about 200 pixels missing just before the 3rd quadrant (upper right), and then the size missing again (resulting in twice the misalignment) just before the 4th quadrant (lower right). These are both apparent in the processed image above. The first effect due to a missing piece can be seen here as well in the raw data for upper quadrant: image