Opened 17 years ago

Last modified 17 years ago

#1012 closed defect (fixed)

libecwj2-3.3 bug with m_nNextLine?

Reported by: warmerdam Owned by: warmerdam
Priority: high Milestone:
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords:
Cc:

Description

 

Attachments (1)

test.cpp (997 bytes ) - added by tom.lynch@… 17 years ago.
sample program

Download all attachments as: .zip

Change History (4)

comment:1 by warmerdam, 17 years ago

When GDAL is built with libecwj2-3.3 from the ERMapper web site, built
with VS7.0 on win32, I get problems in the following scenario:

gdal_translate RGBImage.ecw out.tif

Internally;

 o SetView() for whole image on band 1.
 o Read all lines with ReadLineBIL(). 
 o SetView() for whole image on band 2. 
 o call ReadLineBIL() .. it fails. 

It *seems* that for ECW files the SetView() method
on the NCSJP2FileView class may not reset m_nNextLine
member variable.  In my case this causes the next
ReadLineBIL() call to fail because m_nNextLine is
still pointing beyond the end of the current window. 

Particular points which may be an issue. 
 o The new view was the same as the old view.  The only
   real change was the band list. 
 o The view was large enough to force things through the
   "tiled view" logic. 
 o The ReadLineBIL() code seems to do a pre-check of the
   m_nNextLine variable, which is not done by some of the 
   other Read functions (like ReadLineRGB())

My patch was in CNCSJP2FileView::SetView() (in NCSJP2FileView.cpp)
and I changed code circa line 956 from:

		if(Error == NCS_SUCCESS) {
			m_bHaveValidSetView = true;
		} else {
			m_bHaveValidSetView = false;
		}
		UpdateFileViewInfo();
		return(Error);


to:

		if(Error == NCS_SUCCESS) {
                        m_nNextLine = 0;
			m_bHaveValidSetView = true;
		} else {
			m_bHaveValidSetView = false;
		}
		UpdateFileViewInfo();
		return(Error);

Essentially, if the ecw case of SetView() succeeds, 
ensure the m_nNextLine is reset. 


comment:2 by angus.lau@…, 17 years ago

I should note, I was using libecwj2 3.3 RC1, which I believe to be the 
most current public code.

comment:3 by tom.lynch@…, 17 years ago

I repeated the problem using the attached sample program, and was able to see
that the m_nNextLine value was not reset between SetViews as it should have
been, causing ReadLineBIL to always return NCSECW_READ_FAILED.

The fix Frank suggested seemed logically correct.  I have patched it as 

if(m_pECWFileView) {
		CNCSError Error;
		m_nNextLine = 0;

...

in the code from line 942 (rather than using Frank's fix) since the JP2 case
does not check to see if the SetView is successful before resetting the value.

Simon and I have discussed the fix and can't foresee any regressions it might
cause, as every SetView invalidates the old m_nNextLine value in any case.

by tom.lynch@…, 17 years ago

Attachment: test.cpp added

sample program

Note: See TracTickets for help on using tickets.