possible bug in toolbox? crash in gradHist + proposed fix

[shmulik509]

Piotr,    

I have downloaded your very useful toolbox and after successfully applying the detector I examined the code, thinking of porting it to fully c++.

After adding some new-lines to gradHist() in CalcGradient.cpp, so I can read and hopefully also understand the code, I realized the following is used with a potential bug:


GH(H,ma,mb) H1=H; STRu(*H1,ADD(LDu(*H1),MUL(ma,mb)));


where STRu stores 4 values, although only 2 are really needed. one may argue no harm is done since the upper 2 values are not altered. However, it may occur that the code will try to update at the very end of the allocated memory, and thus will experience an exception when the two excessive values are out of the allowed address range, if the memory chunk is not committed. 


I suggests using:

GH(H,ma,mb) H1_64=(__m64*)(H); H1=H; STRlow(*H1_64,ADD(LDu(*H1),MUL(ma,mb)));


with:

RETf STRlow( __m64 &x, const __m128 y ) { _mm_storel_pi(&x,y); return y; }


Best Regards,

Shmulik

[jmbuena]

The proposal does not fix the bug:

Matlab 2016b in debian linux still crashes when I use this script:

I = imread('peppers.png');
sz1 = [4922, 3938];

for i=1:10 % After some iterations crash (when deallocating O?)
disp(i);
I=imresize(I,sz1);
I = rgbConvert(I, 'luv');
[M,O] = gradientMag(I, 0, 5, 0.0050, 0);
H1 = gradientHist(M, O, 2, 6, 1, 0, 0.2, 0);
end
figure; imshow(I);

[shmulik-akerman]

True. After posting the above I realized that too. The code above only handles the problem of storing two excessive values, but it still accesses illegal location when reading the values. I don't think I have this code anymore, but maybe you can write it yourself using _mm_loadl_pi (https://msdn.microsoft.com/en-us/library/s57cyak2(v=vs.90).aspx) to replace LDu with your own LDlow. I'll look into my disks to see if I can find that.

[jmbuena]

I'm afraid I don't have this low level programming skills :-(.
In any case, thank you very much for looking into it.

[shmulik509]

1. your code above really abuses the image I. you may wish to assign the luv to some other name...
2. I don't think I experienced actual crashes from the bug I mention above, but maybe your crashes relate to fact your image width does not divide by 4, or something like that
3. I'm afraid I don't have this code anymore, nor the environment to test it, but again, I suspect your issue is different. if you were hit by the above bug your would have experienced access violation, as the original code tries to access space beyond its own, without changing the content, though

[jmbuena]

You are right in the use of variable I, but it is just a quick and dirty test.

I think the issue it is related at least with the size of the images, but could be the size not dividing by 4 (not checked it). In any case, I have been able to workaround the problem (a quick and dirty hack) in my case by resizing the images when they are too big (at least training with AFLW database) by changing acfDetectImg in the acfDetect.m file:

function bbs = acfDetectImg( I, detector )
% JMBUENA: Workaround for BUG in gradientHist with big images
sz = size(I);
old_w = sz(2);
old_h = sz(1);
big_image = 0;
big_size = 4100*3500*3; % 4100*3500 pixels x 3 channels ~ 41 MBytes (1 byte/channel)
max_size = 3500;
if  (prod(sz) > big_size)
  big_image = 1;
  if (old_h == max(sz))
    new_w = round(max_size * (old_w/old_h));
    new_h = max_size;
  else
    new_h = round(max_size * (old_h/old_w));
    new_w = max_size;
  end
  I = imResample(I, [new_h, new_w]);
end

% Run trained sliding-window object detector on given image.
Ds=detector; if(~iscell(Ds)), Ds={Ds}; end; nDs=length(Ds);
opts=Ds{1}.opts; pPyramid=opts.pPyramid; pNms=opts.pNms;
imreadf=opts.imreadf; imreadp=opts.imreadp;
shrink=pPyramid.pChns.shrink; pad=pPyramid.pad;
separate=nDs>1 && isfield(pNms,'separate') && pNms.separate;
% read image and compute features (including optionally applying filters)
if(all(ischar(I))), I=feval(imreadf,I,imreadp{:}); end
P=chnsPyramid(I,pPyramid); bbs=cell(P.nScales,nDs);
if(isfield(opts,'filters') && ~isempty(opts.filters)), shrink=shrink*2;
  for i=1:P.nScales, fs=opts.filters; C=repmat(P.data{i},[1 1 size(fs,4)]);
    for j=1:size(C,3), C(:,:,j)=conv2(C(:,:,j),fs(:,:,j),'same'); end
    P.data{i}=imResample(C,.5);
  end
end
% apply sliding window classifiers
for i=1:P.nScales
  for j=1:nDs, opts=Ds{j}.opts;
    modelDsPad=opts.modelDsPad; modelDs=opts.modelDs;
    bb = acfDetect1(P.data{i},Ds{j}.clf,shrink,...
      modelDsPad(1),modelDsPad(2),opts.stride,opts.cascThr);
    shift=(modelDsPad-modelDs)/2-pad;
    bb(:,1)=(bb(:,1)+shift(2))/P.scaleshw(i,2);
    bb(:,2)=(bb(:,2)+shift(1))/P.scaleshw(i,1);
    bb(:,3)=modelDs(2)/P.scales(i);
    bb(:,4)=modelDs(1)/P.scales(i);
    if(separate), bb(:,6)=j; end; bbs{i,j}=bb;
  end
end; bbs=cat(1,bbs{:});
if(~isempty(pNms)), bbs=bbNms(bbs,pNms); end

% JMBUENA: AVOID BUG of gradientHist with too big images.
if (big_image)
  bbs(:,1) = round(bbs(:,1) * (old_w/new_w));
  bbs(:,2) = round(bbs(:,2) * (old_h/new_h));
  bbs(:,3) = round(bbs(:,3) * (old_w/new_w));
  bbs(:,4) = round(bbs(:,4) * (old_h/new_h));
end
end

Hope this will be useful to somebody.