#2927 closed defect (fixed)
ScanJPEGBlocks() finds too many "Start of Image" markers
Reported by: | cobalto | Owned by: | warmerdam |
---|---|---|---|
Priority: | normal | Milestone: | 1.6.1 |
Component: | default | Version: | unspecified |
Severity: | major | Keywords: | nitf |
Cc: |
Description
in NITFDataset::ScanJPEGBlocks(), the entire file is scanned to find every instance of marker 0xff,0xd8. However, these bytes can also occur by chance inside of application-specific data payload sections, where they should not be interpreted as block starts. I made a crude patch to search for these application-specific sections, and ignore their data during the block scan.
my patch replaces
for( i = 0; i < nReadSize-1; i++ ) { if( abyBlock[i] == 0xff && abyBlock[i+1] == 0xd8 ) { CPLAssert( iNextBlock <= psImage->nBlocksPerRow*psImage->nBlocksPerColumn ); panJPEGBlockOffset[iNextBlock++] = panJPEGBlockOffset[0] + iSegOffset + i; } }
with
for( i = 0; i < nReadSize-1; i++ ) { if (ignoreBytes == 0) { if( abyBlock[i] == 0xff ) { if ( abyBlock[i+1] == 0xd8 ) { CPLAssert( iNextBlock <= psImage->nBlocksPerRow*psImage->nBlocksPerColumn ); panJPEGBlockOffset[iNextBlock++] = panJPEGBlockOffset[0] + iSegOffset + i; } else if ( abyBlock[i+1] >= 0xe0 && abyBlock[i+1] < 0xf0 ) { ignoreBytes = -2; } } } else if (ignoreBytes < 0) { if (ignoreBytes == -1) ignoreBytes = abyBlock[i]*256 + abyBlock[i+1]; else ignoreBytes++ } else { ignoreBytes--; } }
where ignoreBytes = 0; is defined before the while loop.
Change History (4)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Keywords: | nitf added |
---|---|
Milestone: | → 1.6.1 |
Resolution: | → fixed |
Status: | new → closed |
I've commited in trunk (r16709) and branches/1.6 (r16710) a variation of your patch that hopefully should be equivalent and a bit more readable ;-) I've tested that it works with the nitf_online_9 test that contains an application data marker. I put a "0xff 0xd8 0xff 0xc2" sequence inside the application data. Without the patch, the decoding fails. With it, this is properly skipped.
comment:3 by , 15 years ago
One functional difference in your patch concerns me: it appears to skip over at most nReadBytes after the app data marker. What happens if the app data payload is larger than nReadBytes AND a "0xff 0xd8" occurs towards the end of the app data?
I should also add that the additional start-of-image markers are interpreted as JPEG block beginnings, and completely mess up the block indexing of all blocks ocurring after an extraneous marker.
Also note that the JPEG standard defines 0xff,0xe? as application-specific data markers, which are followed by a 2-byte payload length.