Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#5172 closed enhancement (fixed)

[PATCH] Add CreateCopy() support for the Farsite LCP driver.

Reported by: Kyle Shannon Owned by: Kyle Shannon
Priority: normal Milestone: 1.11.0
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords: lcpdataset
Cc: Even Rouault

Description

It would be nice to have a method to create lcp datasets using gdal. All units/metadata should be configurable using creation options, with reasonable defaults. I will prepare a patch and apply after some testing.

Attachments (1)

lcp_createcopy.patch (34.8 KB) - added by Kyle Shannon 4 years ago.
updated (final) patch.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by Kyle Shannon

Milestone: 1.10.12.0

Push to 2.0/trunk and attach patch.

comment:2 Changed 4 years ago by Kyle Shannon

Update patch that classifies the band data on request.

Changed 4 years ago by Kyle Shannon

Attachment: lcp_createcopy.patch added

updated (final) patch.

comment:3 Changed 4 years ago by Kyle Shannon

Cc: Even Rouault added
Summary: Add CreateCopy() support for the Farsite LCP driver.[PATCH] Add CreateCopy() support for the Farsite LCP driver.

Even, I am sure it would be okay to apply this given the small user-base, but could you look over the patch including the byte swapping, it's my first shot at it. No rush whatsoever. Thanks.

comment:4 Changed 4 years ago by Even Rouault

Kyle,

just a few things I've caught :

    1475    #ifdef CPL_MSB 
    1476        GDALSwapWords( panMetadata, 4, LCP_MAX_BANDS, 4 ); 
    1477    #endif 
    1478        VSIFWriteL( panMetadata, 2, LCP_MAX_BANDS, fp );

--> Should be GDALSwapWords( panMetadata, 2, LCP_MAX_BANDS, 2 ); I think

    1482        const char *pszFilePath = 
    1483            CPLFormFilename( CPLGetPath( papszFileList[0] ), 
    1484                             CPLGetBasename( papszFileList[0] ), 
    1485                             CPLGetExtension( papszFileList[0] ) );

--> unless I've missed something, const char *pszFilePath = papszFileList[0] should be equivalent, no ?

1489 #ifdef CPL_MSB 1490 GDALSwapWords( pszFilePath, 1, CPLStrnlen( pszFilePath ), 1 ); 1491 #endif

--> useless. Byte swapping only makes sense for multi-byte datatypes.

1511 #ifdef CPL_MSB 1512 GDALSwapWords( pszDescription, 1, CPLStrlen( pszDescription ), 1 ); 1513 #endif

--> idem

1529 if( !pfnProgress( 0.0, NULL, pProgressData ) ) 1530 { 1531 VSIFCloseL( fp ); 1532 return NULL; 1533 }

--> lacks a VSIFree( panScanline );

1558 if( !pfnProgress( iLine / (double)nYSize, NULL, pProgressData ) ) 1559 { 1560 VSIFCloseL( fp ); 1561 return NULL; 1562 }

--> idem

It would be good if you could enhance autotest/gdrivers/lcp.py to test this new write capability.

Other than the above, just go ahead and apply your code.

comment:5 in reply to:  4 Changed 4 years ago by Kyle Shannon

Replying to rouault:

Kyle,

just a few things I've caught :

    1475    #ifdef CPL_MSB 
    1476        GDALSwapWords( panMetadata, 4, LCP_MAX_BANDS, 4 ); 
    1477    #endif 
    1478        VSIFWriteL( panMetadata, 2, LCP_MAX_BANDS, fp );

--> Should be GDALSwapWords( panMetadata, 2, LCP_MAX_BANDS, 2 ); I think

Yes. Corrected.

    1482        const char *pszFilePath = 
    1483            CPLFormFilename( CPLGetPath( papszFileList[0] ), 
    1484                             CPLGetBasename( papszFileList[0] ), 
    1485                             CPLGetExtension( papszFileList[0] ) );

--> unless I've missed something, const char *pszFilePath = papszFileList[0] should be equivalent, no ?

Yes, corrected.

1489 #ifdef CPL_MSB 1490 GDALSwapWords( pszFilePath, 1, CPLStrnlen( pszFilePath ), 1 ); 1491 #endif

--> useless. Byte swapping only makes sense for multi-byte datatypes.

Yes, Corrected. My mistake.

1511 #ifdef CPL_MSB 1512 GDALSwapWords( pszDescription, 1, CPLStrlen( pszDescription ), 1 ); 1513 #endif

--> idem

1529 if( !pfnProgress( 0.0, NULL, pProgressData ) ) 1530 { 1531 VSIFCloseL( fp ); 1532 return NULL; 1533 }

--> lacks a VSIFree( panScanline );

Yes, corrected.

1558 if( !pfnProgress( iLine / (double)nYSize, NULL, pProgressData ) ) 1559 { 1560 VSIFCloseL( fp ); 1561 return NULL; 1562 }

--> idem

It would be good if you could enhance autotest/gdrivers/lcp.py to test this new write capability.

I have about 10 new tests written. I will write some to handle creation options and proper copying.

Other than the above, just go ahead and apply your code.

Thanks for taking a look.

comment:6 Changed 4 years ago by Even Rouault

Resolution: fixed
Status: newclosed

Look like this ticket could be closed since r26284 , r26285 and r26286

comment:7 Changed 4 years ago by Kyle Shannon

Move check for linear unit into a different control statement so it is checked all the time (trunk r26396)

comment:8 Changed 4 years ago by Even Rouault

Milestone: 2.01.11.0
Note: See TracTickets for help on using tickets.