Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch to read GML geo metadata and other associated data for inclusio… #1110

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Patch to read GML geo metadata and other associated data for inclusio… #1110

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
53df2bf
Patch to read GML geo metadata and other associated data for inclusio…
HWiman Mar 22, 2018
2c8e3b4
Example how to dump associated data, if there is any.
HWiman Mar 22, 2018
dfcb751
Fixes after code review by rouault.
HWiman Mar 22, 2018
64a8e4e
Fixed indentation using astyle=Kernighan&Ritchie
HWiman Mar 22, 2018
ca773e4
Eoln set to LF only (not CR-LF)
HWiman Mar 22, 2018
4dc8a20
Reinserted comment that was "lost in translation"
HWiman Mar 23, 2018
713e131
Run astyle
rouault Mar 28, 2018
ae71bb8
Merge pull request #1 from rouault/associated_data_support
HWiman Apr 11, 2018
34801b4
- Moved ASOC data to opj_jp2_metadata_t
HWiman May 28, 2018
ec4aec1
Merge branch 'associated_data_support' of https://github.com/HWiman/o…
HWiman May 28, 2018
58c19b6
- Always return metadata struct, but set asoc to null iff j2k.
HWiman May 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/lib/openjp2/j2k.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10442,6 +10442,9 @@ opj_codestream_info_v2_t* j2k_get_cstr_info(opj_j2k_t* p_j2k)
if (!cstr_info) {
return NULL;
}

cstr_info->nbasoc = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation issue. Please build with cmake -DWITH_ASTYLE=ON and run scripts/prepare-commit.sh

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just did that, but with no effect. Is the prepare-commit.sh Windows-compatible?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum probably not... Maybe under Cygwin ? Anyway on already committed code, this has no effect. You can use scripts/astyle.sh your.c to fix an existing committed file

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find an exact match of astyle in Code::Blocks. The closest I got was Kernighan&Ritchie but had to make small edits in source files manually. I guess the
build system checks if this will pass?
astyle_kr

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest push clearly changed everything. My attempt with astyle failed completely.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid I need your assistance to format the committed six files since it can not be done on Windows. I compiled on a remote Linux machine WITH_ASTYLE, but it does not have C++11 and I am not authorized to install gcc. Can you please run astyle.sh on the commited files?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've run astyle. Please pull from rouault:associated_data_support to get rouault@713e131

cstr_info->asoc_info = 00;

cstr_info->nbcomps = p_j2k->m_private_image->numcomps;

Expand Down
164 changes: 159 additions & 5 deletions src/lib/openjp2/jp2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ static OPJ_BOOL opj_jp2_read_cdef(opj_jp2_t * jp2,
static void opj_jp2_apply_cdef(opj_image_t *image, opj_jp2_color_t *color,
opj_event_mgr_t *);

/**
* Destroy list of ASOC entities
*/
static void opj_jp2_asoc_destroy( opj_jp2_asoc_t *p_asoc, OPJ_UINT32 num );

/**
* Writes the Channel Definition box.
*
Expand Down Expand Up @@ -159,7 +164,23 @@ static OPJ_BOOL opj_jp2_write_ftyp(opj_jp2_t *jp2,
static OPJ_BOOL opj_jp2_read_ftyp(opj_jp2_t *jp2,
OPJ_BYTE * p_header_data,
OPJ_UINT32 p_header_size,
opj_event_mgr_t * p_manager);
opj_event_mgr_t * p_manager);

/**
* Reads a ASOC box - Associated data box.
* Also reads contained label (LBL) and XML boxes
*
* @param p_header_data the data contained in the ASOC box.
* @param jp2 the jpeg2000 file codec.
* @param p_header_size the size of the data contained in the ASOC box.
* @param p_manager the user event manager.
*
* @return true if the ASOC box is valid.
*/
static OPJ_BOOL opj_jp2_read_asoc( opj_jp2_t *jp2,
OPJ_BYTE * p_header_data,
OPJ_UINT32 p_header_size,
opj_event_mgr_t * p_manager);

static OPJ_BOOL opj_jp2_skip_jp2c(opj_jp2_t *jp2,
opj_stream_private_t *stream,
Expand Down Expand Up @@ -425,7 +446,8 @@ static const opj_jp2_header_handler_t * opj_jp2_find_handler(OPJ_UINT32 p_id);
static const opj_jp2_header_handler_t jp2_header [] = {
{JP2_JP, opj_jp2_read_jp},
{JP2_FTYP, opj_jp2_read_ftyp},
{JP2_JP2H, opj_jp2_read_jp2h}
{JP2_JP2H, opj_jp2_read_jp2h},
{JP2_ASOC, opj_jp2_read_asoc}
};

static const opj_jp2_header_handler_t jp2_img_header [] = {
Expand Down Expand Up @@ -2636,6 +2658,104 @@ static OPJ_BOOL opj_jp2_read_ftyp(opj_jp2_t *jp2,
jp2->jp2_state |= JP2_STATE_FILE_TYPE;

return OPJ_TRUE;
}

static OPJ_BOOL opj_jp2_read_asoc( opj_jp2_t *jp2,
OPJ_BYTE * p_header_data,
OPJ_UINT32 p_header_size,
opj_event_mgr_t * p_manager)
{
OPJ_UINT32 label_tag;
OPJ_UINT32 asoc_tag;
OPJ_UINT32 asoc_size;
opj_jp2_asoc_t *asoc;

/* preconditions */
assert(jp2 != 00);
assert(p_header_data != 00);
assert(p_manager != 00);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check p_header_size is at least 8 bytes long here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

opj_read_bytes(p_header_data,&asoc_size,4);
p_header_data += 4;
p_header_size -= 4;

opj_read_bytes(p_header_data,&label_tag,4);
p_header_data += 4;
p_header_size -= 4;
asoc_size -= 4;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should validate asoc_size is >= 4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and probably verify asoc_size against p_header_size to avoid excessive memory allocation attempt

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


if (label_tag != JP2_LBL) {
/* TODO: Verify that ASOC must have a following label ? */
opj_event_msg(p_manager, EVT_WARNING, "ASOC data does not have a label (LBL)\n");
return OPJ_TRUE; // No error if we could not parse
}

if ( jp2->numasoc == 0 ) {
jp2->numasoc = 1;
jp2->asoc = opj_malloc(sizeof(opj_jp2_asoc_t));
}
else {
(jp2->numasoc)++;
jp2->asoc = opj_realloc(jp2->asoc, jp2->numasoc * sizeof(opj_jp2_asoc_t));
}
asoc = &(jp2->asoc[jp2->numasoc-1]);
asoc->level = jp2->numasoc-1; /* TODO: This is not correct if a parent asoc contains multiple child asocs! */
asoc->label_length = asoc_size;
asoc->label = opj_malloc(asoc_size);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be safer for users to allocate asoc_size + 1 and nul terminate asoc->label[asoc_size]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check asoc->label is not null

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asoc->label is always uninitialized since it is created a few lines above.

memcpy(asoc->label, p_header_data, asoc_size);
asoc->xml_buf = 00;
asoc->xml_len = 0;

p_header_data += asoc_size;
p_header_size -= asoc_size;

opj_read_bytes(p_header_data,&asoc_tag,4);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ypi sjpimd check that there's at least 4 remaining bytes

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

p_header_data += 4;
p_header_size -= 4;

switch (asoc_tag) {
case JP2_ASOC: {
/* Start of nested ASOC tags. Parse this level. */
if (!opj_jp2_read_asoc( jp2, p_header_data, p_header_size, p_manager )) {
return OPJ_FALSE;
}
}
break;

case JP2_XML: {
asoc->xml_len = p_header_size;
asoc->xml_buf = opj_malloc(p_header_size);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null pointer check needed.
Extra byte to force nul-termination would be safer

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asoc->xml_buf is always uninitialized since it is created a few lines above.
NULL termination fixed

memcpy( asoc->xml_buf, p_header_data, p_header_size );
}
break;

default: {
/* Copy the unknown data for external handling.
NOTE: This is not tested, but does the same as if an XML tag was found.*/
asoc->xml_len = p_header_size;
asoc->xml_buf = opj_malloc(p_header_size);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

memcpy( asoc->xml_buf, p_header_data, p_header_size );
}
}

return OPJ_TRUE;
}

static void opj_jp2_asoc_destroy( opj_jp2_asoc_t *p_asoc, OPJ_UINT32 num )
{
OPJ_UINT32 i;
opj_jp2_asoc_t *asoc;
for (i=0; i<num; i++)
{
asoc = &(p_asoc[i]);
opj_free( asoc->label );
asoc->label = 00;
asoc->label_length = 0;

opj_free( asoc->xml_buf );
asoc->xml_buf = 00;
asoc->xml_len = 0;
}
}

static OPJ_BOOL opj_jp2_skip_jp2c(opj_jp2_t *jp2,
Expand Down Expand Up @@ -3068,6 +3188,12 @@ void opj_jp2_destroy(opj_jp2_t *jp2)
if (jp2->m_procedure_list) {
opj_procedure_list_destroy(jp2->m_procedure_list);
jp2->m_procedure_list = 00;
}

if ( jp2->numasoc ) {
opj_jp2_asoc_destroy( jp2->asoc, jp2->numasoc );
jp2->asoc = 00;
jp2->numasoc = 0;
}

opj_free(jp2);
Expand Down Expand Up @@ -3204,7 +3330,12 @@ opj_jp2_t* opj_jp2_create(OPJ_BOOL p_is_decoder)
if (! jp2->m_procedure_list) {
opj_jp2_destroy(jp2);
return 00;
}
}

/* Association data */
jp2->asoc = 00;
jp2->numasoc = 0;

}

return jp2;
Expand All @@ -3226,8 +3357,31 @@ opj_codestream_index_t* jp2_get_cstr_index(opj_jp2_t* p_jp2)
}

opj_codestream_info_v2_t* jp2_get_cstr_info(opj_jp2_t* p_jp2)
{
return j2k_get_cstr_info(p_jp2->j2k);
{
opj_codestream_info_v2_t* p_info = j2k_get_cstr_info(p_jp2->j2k);
jp2_copy_asoc_data( p_jp2, p_info );
return p_info;
}

OPJ_BOOL jp2_copy_asoc_data( opj_jp2_t* p_jp2, opj_codestream_info_v2_t* p_info )
{
OPJ_UINT32 i;
opj_jp2_asoc_t *asoc, *to_asoc;
p_info->nbasoc = p_jp2->numasoc;
p_info->asoc_info = opj_malloc(p_info->nbasoc * sizeof(opj_jp2_asoc_t));
for (i=0; i<p_info->nbasoc; i++) {
asoc = &(p_jp2->asoc[i]);
to_asoc = &(p_info->asoc_info[i]);
to_asoc->level = asoc->level;
to_asoc->label_length = asoc->label_length;
to_asoc->xml_len = asoc->xml_len;
to_asoc->label = opj_malloc( to_asoc->label_length );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null pointer checks needed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

memcpy(to_asoc->label, asoc->label, to_asoc->label_length);
to_asoc->xml_buf = opj_malloc( to_asoc->xml_len);
memcpy(to_asoc->xml_buf, asoc->xml_buf, to_asoc->xml_len);
}

return OPJ_TRUE;
}

OPJ_BOOL opj_jp2_set_decoded_resolution_factor(opj_jp2_t *p_jp2,
Expand Down
17 changes: 14 additions & 3 deletions src/lib/openjp2/jp2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@
#define JP2_CDEF 0x63646566 /**< Channel Definition box */
#define JP2_DTBL 0x6474626c /**< Data Reference box */
#define JP2_BPCC 0x62706363 /**< Bits per component box */
#define JP2_JP2 0x6a703220 /**< File type fields */
#define JP2_JP2 0x6a703220 /**< File type fields */
#define JP2_ASOC 0x61736f63 /**< Associated data */
#define JP2_LBL 0x6c626c20 /**< Association label */
#define JP2_XML 0x786d6c20 /**< XML data */

/* For the future */
/* #define JP2_RES 0x72657320 */ /**< Resolution box (super-box) */
Expand Down Expand Up @@ -184,7 +187,10 @@ typedef struct opj_jp2 {
OPJ_UINT32 jp2_state;
OPJ_UINT32 jp2_img_state;

opj_jp2_color_t color;
opj_jp2_color_t color;

opj_jp2_asoc_t *asoc;
OPJ_UINT32 numasoc;

OPJ_BOOL ignore_pclr_cmap_cdef;
OPJ_BYTE has_jp2h;
Expand Down Expand Up @@ -478,7 +484,12 @@ void jp2_dump(opj_jp2_t* p_jp2, OPJ_INT32 flag, FILE* out_stream);
*
*@return the codestream information extract from the jpg2000 codec
*/
opj_codestream_info_v2_t* jp2_get_cstr_info(opj_jp2_t* p_jp2);
opj_codestream_info_v2_t* jp2_get_cstr_info(opj_jp2_t* p_jp2);

/**
* Copy associated data
*/
OPJ_BOOL jp2_copy_asoc_data( opj_jp2_t* p_jp2, opj_codestream_info_v2_t* p_info );

/**
* Get the codestream index from a JPEG2000 codec.
Expand Down
51 changes: 50 additions & 1 deletion src/lib/openjp2/openjpeg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#include "opj_includes.h"

static void opj_asoc_destroy( opj_jp2_asoc_t *p_asoc, OPJ_UINT32 num );

/* ---------------------------------------------------------------------- */
/* Functions to set the message handlers */
Expand Down Expand Up @@ -950,6 +951,26 @@ void OPJ_CALLCONV opj_dump_codec(opj_codec_t *p_codec,
/* TODO return error */
/* fprintf(stderr, "[ERROR] Input parameter of the dump_codec function are incorrect.\n"); */
return;
}

void OPJ_CALLCONV opj_dump_associated_data(
opj_codestream_info_v2_t* cstr_info,
FILE* output_stream )
{
OPJ_UINT32 i;
if ( cstr_info->asoc_info ) {
fprintf(output_stream, "\n\nAssociated data: {\n");
for (i=0; i<cstr_info->nbasoc; i++) {
fprintf(output_stream, "\tlabel=%s, xml/data=", (char*) cstr_info->asoc_info[i].label);
if (cstr_info->asoc_info[i].xml_buf) {
fprintf(output_stream, "%s\n", (char*) cstr_info->asoc_info[i].xml_buf);
}
else {
fprintf(output_stream, "NULL\n");
}
}
fprintf(output_stream, "}\n");
}
}

opj_codestream_info_v2_t* OPJ_CALLCONV opj_get_cstr_info(opj_codec_t *p_codec)
Expand All @@ -963,6 +984,23 @@ opj_codestream_info_v2_t* OPJ_CALLCONV opj_get_cstr_info(opj_codec_t *p_codec)
return NULL;
}

void opj_asoc_destroy( opj_jp2_asoc_t *p_asoc, OPJ_UINT32 num )
{
OPJ_UINT32 i;
opj_jp2_asoc_t *asoc;
for (i=0; i<num; i++)
{
asoc = &(p_asoc[i]);
opj_free( asoc->label );
asoc->label = 00;
asoc->label_length = 0;

opj_free( asoc->xml_buf );
asoc->xml_buf = 00;
asoc->xml_len = 0;
}
}

void OPJ_CALLCONV opj_destroy_cstr_info(opj_codestream_info_v2_t **cstr_info)
{
if (cstr_info) {
Expand All @@ -975,6 +1013,12 @@ void OPJ_CALLCONV opj_destroy_cstr_info(opj_codestream_info_v2_t **cstr_info)
/* FIXME not used for the moment*/
}

if ((*cstr_info)->nbasoc) {
opj_asoc_destroy((*cstr_info)->asoc_info, (*cstr_info)->nbasoc);
(*cstr_info)->asoc_info = 00;
(*cstr_info)->nbasoc = 0;
}

opj_free((*cstr_info));
(*cstr_info) = NULL;
}
Expand Down Expand Up @@ -1049,7 +1093,12 @@ opj_stream_t* OPJ_CALLCONV opj_stream_create_file_stream(

return l_stream;
}


OPJ_OFF_T opj_stream_skip_api(opj_stream_t * p_stream, OPJ_OFF_T p_size)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated to this pull request AFAICS. Better put that in another one

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, sorry! Some files (like NITF) encapsulate JP2 in a file, so we need to offset the start of file reading. I did not find any way to do that so added this method.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit self-promotion, but you are probably aware of the GDAL library (http://gdal.org/) that takes care of all this geo stuff already and deals with NITF.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that´s an impressing and complete library! We do all our software in Java so it would require a lot of JNI bindings and we have implemented almost all image and sensor support we need already, this GML parsing being one of few missing parts.

{
opj_stream_private_t* l_stream = (opj_stream_private_t*) p_stream;
return l_stream->m_opj_skip(l_stream, p_size, NULL);
}

void* OPJ_CALLCONV opj_image_data_alloc(OPJ_SIZE_T size)
{
Expand Down
Loading