Matlab Computer Vision

The code below takes a video as input (a drop onto a fabric surface) and I want it to return the area of the drop.

Currently the code works, but I'm no Matlab expert, and wrote it entirely through google/hack&slashing my way into a successful form. Unfortunately it's slow, and it's probably not written with good style.

I was hoping that anyone would be able to tell me how I can improve the code. I don't think that you'll need a video file or calibration file, but if it would help I can include it. I am also curious whether there is a better way to output the data to an XLS file than writing in the title/values.

I'm mainly concerned about style and efficiency.

`%profile off
%profile clear

clear all;
set(0,'ShowHiddenHandles','on')
delete(get(0,'Children')) % Delete any currently open figures
%profile on

%Input user specified filename
[filename, pathname] = uigetfile('*.avi', 'Select a video file');

if isequal(filename,0)
disp('User selected Cancel')
else
disp(['User selected: ', fullfile(pathname, filename)]);
end
% [~,~,ext]=fileparts(filename);
dataFileName={filename};
nab=fullfile(pathname, filename);
[filename, pathname] = uigetfile({'*.xlsx'},'Select the calibration file'); % choose name and location of Excel calibration file
if isequal(filename,0)
disp('User selected Cancel');
else
disp(['User selected: ', fullfile(pathname, filename)]);
end
[pathstr,name,ext]=fileparts(filename);
calfile=fullfile(pathname, filename);

% read the calibration file specifically

interval = xlsread(calfile,'Calibration','B1');
repeats = xlsread(calfile,'Calibration','B2');
pixels_per_mm = xlsread(calfile,'Calibration','B8');
stainValue = xlsread(calfile,'Calibration','B10');

maxFrames = xlsread(calfile,'Calibration','B23');
maxstainNum = xlsread(calfile,'Calibration','B24');

densityBlood = xlsread(calfile,'Calibration','B41');
surftBlood = xlsread(calfile,'Calibration','B42');
densityAir = xlsread(calfile,'Calibration','B35') ;
visAi

There's a lot going on in this script, but I've tried to cover all of the important parts.

Style and conventions:

I think your coding style is very good.

• You're using nice a descriptive variable names - Good!

• Your variable names are consitent (camelCase) - Good!

• (Except pixels_per_mm)

• You're using spaces - Good!

• (Most of the places, [pathstr,name,ext]=fileparts(filename);)

• You're not using i and j as variable names in loops - Good!

• You're using .' and not ' when transposing - Good!

Writing to Excel:

xlswrite opens and closes the Excel server everytime you run the command. This can obiously be a huge bottleneck if you run the command in a loop, or many times in a row as you do. You have two alternatives, that are both quite good.

You are writing the individual headers first, followed by the individual columns with data. This can be done much simpler, either by first writing all headers, then all data like this:

headers = {'Calibration Data', 'Filename'}
allData = {col1, col2, col3}
xlswrite(stainresults, headers, sheetref, 'A1')
xlswrite(stainresults, allData, sheetref, 'A2')

or, put everything into a single cell array, and print it in one singe command:

headersAndData = {headers; allData};
xlswrite(stainresults, headersAndData, sheetref, 'A1')

The other alternative is to use xlswrite1 from the file exchange. It's a very good and versatile function that does everything xslwrite does. The only difference is that xlswrite1 keeps Excel open until you're finished writing to it. This makes it a lot faster than xlswrite when used in loops. This approach will be the simplest if you don't want to rewrite your code.

Reading from Excel:

You're reading one column at a time. Don't! Read all columns into cells in one go, and, if necessary split it into separate variables in Matlab afterwards:

[col1, col2, col3] = allData{:};

It's much better to keep it as one variable though and use indices when necessary. It's faster, and it's "the Matlab way".

Loops:

Loops such as this one should be rewritten:

for n=1:nframes
    if n < tsFrame
        Time(n) = 0;
    else
        Time(n) = interval*(kCol(n)-dsFrame);
    end
end

First off: Pre-allocate memory for vectors that are used in loops, so in front of a loop like this you should always do: Time = zeros(nframes, 1);. This is much faster. The Matlab editor even warns you about it. But, in this case, the loop isn't needed at all!

Time = interval.*(kCol - dsFrame);
Time(1:floor(tsFrame)) = 0;   % or Time(1:tsFrame) = 0; if tsFrame is an integer

Extracting data from struct:

All the lines below can be simplified by converting props into a cell array:

stainData(stain,3) = props(stain).FilledArea;          
stainData(stain,4) = props(stain).Eccentricity;

Simply do:

stainData = struct2cell(props(:))

This will create a cell array stainData with numObj columns, and rows corresponding to the fields in props. You might need to tweak some other parts of the code to make it exactly the way you want it (since you have props.Centroid(1) and props.Centroid(2), but that should be very simple.

StackExchange Code Review Q#112381, answer score: 5