From 5540ec23ecc79b90bb5b6eeab318fdd1f79465cf Mon Sep 17 00:00:00 2001 From: Stefan Kost Date: Fri, 20 Mar 2009 15:18:05 +0200 Subject: [PATCH] metdata: more logging and code cleanups Sprinkle more debug log statements into the code. Move some repeaded string constant into header files and use sizeof instead of manually counted bytes. Add comments. --- ext/metadata/metadataexif.h | 6 ++++ ext/metadata/metadataiptc.h | 6 ++++ ext/metadata/metadatamuxjpeg.c | 61 +++++++++++++++++--------------- ext/metadata/metadatamuxpng.c | 18 ++++++---- ext/metadata/metadataparsejpeg.c | 60 +++++++++++++++---------------- ext/metadata/metadataparsepng.c | 5 +++ ext/metadata/metadataxmp.h | 5 +++ 7 files changed, 94 insertions(+), 67 deletions(-) diff --git a/ext/metadata/metadataexif.h b/ext/metadata/metadataexif.h index d7047e6902..6040ef4ded 100644 --- a/ext/metadata/metadataexif.h +++ b/ext/metadata/metadataexif.h @@ -50,6 +50,12 @@ G_BEGIN_DECLS +/* + * defines + */ + +#define EXIF_HEADER "Exif\0" + typedef enum { GST_META_EXIF_BYTE_ORDER_MOTOROLA, GST_META_EXIF_BYTE_ORDER_INTEL diff --git a/ext/metadata/metadataiptc.h b/ext/metadata/metadataiptc.h index 4b2cfc0530..843a4925ac 100644 --- a/ext/metadata/metadataiptc.h +++ b/ext/metadata/metadataiptc.h @@ -50,6 +50,12 @@ G_BEGIN_DECLS +/* + * defines + */ +#define PHOTOSHOP_HEADER "Photoshop 3.0" + + /* * external function prototypes */ diff --git a/ext/metadata/metadatamuxjpeg.c b/ext/metadata/metadatamuxjpeg.c index 0c56a606b1..cec049b20c 100644 --- a/ext/metadata/metadatamuxjpeg.c +++ b/ext/metadata/metadatamuxjpeg.c @@ -77,6 +77,7 @@ */ #include "metadatamuxjpeg.h" +#include "metadataxmp.h" #include @@ -203,12 +204,12 @@ metadatamux_jpeg_parse (JpegMuxData * jpeg_data, guint8 * buf, { int ret = META_PARSING_DONE; - guint8 mark[2] = { 0x00, 0x00 }; const guint8 *step_buf = buf; *next_start = buf; if (jpeg_data->state == JPEG_MUX_NULL) { + guint8 mark[2]; if (*bufsize < 2) { GST_INFO ("need more data"); @@ -249,9 +250,8 @@ metadatamux_jpeg_parse (JpegMuxData * jpeg_data, guint8 * buf, } done: - + GST_INFO ("finishing: %d", ret); return ret; - } /* @@ -268,46 +268,43 @@ done: void metadatamux_jpeg_lazy_update (JpegMuxData * jpeg_data) { - gint i = 0; + gsize i; gboolean has_exif = FALSE; + MetadataChunkArray *chunks = jpeg_data->inject_chunks; - while (i < jpeg_data->inject_chunks->len) { - if (jpeg_data->inject_chunks->chunk[i].size > 0 && - jpeg_data->inject_chunks->chunk[i].data) { - switch (jpeg_data->inject_chunks->chunk[i].type) { + GST_INFO ("checking %d chunks", chunks->len); + + for (i = 0; i < chunks->len; ++i) { + + GST_INFO ("checking chunk[%" G_GSIZE_FORMAT "], type=%d, len=%u", + i, chunks->chunk[i].type, chunks->chunk[i].size); + + if (chunks->chunk[i].size > 0 && chunks->chunk[i].data) { + switch (chunks->chunk[i].type) { case MD_CHUNK_EXIF: - metadatamux_wrap_chunk (&jpeg_data->inject_chunks->chunk[i], NULL, 0, - 0xFF, 0xE1); + metadatamux_wrap_chunk (&chunks->chunk[i], NULL, 0, 0xFF, 0xE1); has_exif = TRUE; break; case MD_CHUNK_IPTC: #ifdef HAVE_IPTC - { - if (metadatamux_wrap_iptc_with_ps3 (&jpeg_data->inject_chunks-> - chunk[i].data, &jpeg_data->inject_chunks->chunk[i].size)) { - metadatamux_wrap_chunk (&jpeg_data->inject_chunks->chunk[i], NULL, - 0, 0xFF, 0xED); + if (metadatamux_wrap_iptc_with_ps3 (&chunks->chunk[i].data, + &chunks->chunk[i].size)) { + metadatamux_wrap_chunk (&chunks->chunk[i], NULL, 0, 0xFF, 0xED); } else { GST_ERROR ("Invalid IPTC chunk\n"); - metadata_chunk_array_remove_by_index (jpeg_data->inject_chunks, i); + metadata_chunk_array_remove_by_index (chunks, i); continue; } - } #endif /* #ifdef HAVE_IPTC */ break; case MD_CHUNK_XMP: - { - static const char XmpHeader[] = "http://ns.adobe.com/xap/1.0/"; - - metadatamux_wrap_chunk (&jpeg_data->inject_chunks->chunk[i], - (guint8 *) XmpHeader, sizeof (XmpHeader), 0xFF, 0xE1); - } + metadatamux_wrap_chunk (&chunks->chunk[i], + (guint8 *) XMP_HEADER, sizeof (XMP_HEADER), 0xFF, 0xE1); break; default: break; } } - ++i; } if (!has_exif) { @@ -392,22 +389,28 @@ metadatamux_jpeg_reading (JpegMuxData * jpeg_data, guint8 ** buf, mark[0] = READ (*buf, *bufsize); mark[1] = READ (*buf, *bufsize); + GST_DEBUG ("parsing JPEG marker : 0x%02x%02x", mark[0], mark[1]); + if (mark[0] == 0xFF) { chunk_size = READ (*buf, *bufsize) << 8; chunk_size += READ (*buf, *bufsize); - if (mark[1] == 0xE0) { /* may be JFIF */ + if (mark[1] == 0xE0) { /* APP0 - may be JFIF */ - if (chunk_size >= 16) { - if (*bufsize < 5) { + /* FIXME: whats the 14 ? according to + * http://en.wikipedia.org/wiki/JFIF#JFIF_segment_format + * its the jfif segment without thumbnails + */ + if (chunk_size >= 14 + 2) { + if (*bufsize < sizeof (JfifHeader)) { GST_INFO ("need more data"); - *next_size = (*buf - *next_start) + 5; + *next_size = (*buf - *next_start) + sizeof (JfifHeader); ret = META_PARSING_NEED_MORE_DATA; goto done; } - if (0 == memcmp (JfifHeader, *buf, 5)) { + if (0 == memcmp (JfifHeader, *buf, sizeof (JfifHeader))) { jfif_found = TRUE; } } else { diff --git a/ext/metadata/metadatamuxpng.c b/ext/metadata/metadatamuxpng.c index 3223b34e7f..008acc6fe3 100644 --- a/ext/metadata/metadatamuxpng.c +++ b/ext/metadata/metadatamuxpng.c @@ -344,15 +344,19 @@ void metadatamux_png_lazy_update (PngMuxData * png_data) { gsize i; + MetadataChunkArray *chunks = png_data->inject_chunks; - for (i = 0; i < png_data->inject_chunks->len; ++i) { - if (png_data->inject_chunks->chunk[i].size > 0 && - png_data->inject_chunks->chunk[i].data) { - switch (png_data->inject_chunks->chunk[i].type) { + GST_INFO ("checking %d chunks", chunks->len); + + for (i = 0; i < chunks->len; ++i) { + + GST_INFO ("checking chunk[%" G_GSIZE_FORMAT "], type=%d, len=%u", + i, chunks->chunk[i].type, chunks->chunk[i].size); + + if (chunks->chunk[i].size > 0 && chunks->chunk[i].data) { + switch (chunks->chunk[i].type) { case MD_CHUNK_XMP: - { - metadatamux_wrap_xmp_chunk (&png_data->inject_chunks->chunk[i]); - } + metadatamux_wrap_xmp_chunk (&chunks->chunk[i]); break; default: GST_ERROR ("Unexpected chunk for PNG muxer."); diff --git a/ext/metadata/metadataparsejpeg.c b/ext/metadata/metadataparsejpeg.c index 8e90b5c726..0fe520174a 100644 --- a/ext/metadata/metadataparsejpeg.c +++ b/ext/metadata/metadataparsejpeg.c @@ -82,9 +82,12 @@ #include "metadataparsejpeg.h" #include "metadataparseutil.h" +#include "metadataexif.h" +#include "metadataxmp.h" #ifdef HAVE_IPTC #include +#include "metadataiptc.h" #endif /* @@ -226,9 +229,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf, guint32 * bufsize, const guint32 offset, guint8 ** next_start, guint32 * next_size) { - int ret = META_PARSING_DONE; - guint8 mark[2] = { 0x00, 0x00 }; const guint8 *step_buf = buf; /* step_buf holds where buf starts. this const value will be passed to @@ -240,6 +241,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf, *next_start = buf; if (jpeg_data->state == JPEG_PARSE_NULL) { + guint8 mark[2]; /* only the first time this function is called it will verify the stream type to be sure it is a JPEG */ @@ -259,9 +261,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf, ret = META_PARSING_ERROR; goto done; } - jpeg_data->state = JPEG_PARSE_READING; - } while (ret == META_PARSING_DONE) { @@ -309,9 +309,8 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf, } done: - + GST_INFO ("finishing: %d", ret); return ret; - } /* @@ -389,12 +388,6 @@ metadataparse_jpeg_reading (JpegParseData * jpeg_data, guint8 ** buf, guint16 chunk_size = 0; static const char JfifHeader[] = "JFIF"; - static const unsigned char ExifHeader[] = - { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; -#ifdef HAVE_IPTC - static const char PhotoshopHeader[] = "Photoshop 3.0"; -#endif - static const char XmpHeader[] = "http://ns.adobe.com/xap/1.0/"; *next_start = *buf; @@ -407,12 +400,14 @@ metadataparse_jpeg_reading (JpegParseData * jpeg_data, guint8 ** buf, mark[0] = READ (*buf, *bufsize); mark[1] = READ (*buf, *bufsize); + GST_DEBUG ("parsing JPEG marker : 0x%02x%02x", mark[0], mark[1]); + if (mark[0] == 0xFF) { - if (mark[1] == 0xD9) { /* end of image */ + if (mark[1] == 0xD9) { /* EOI - end of image */ ret = META_PARSING_DONE; jpeg_data->state = JPEG_PARSE_DONE; goto done; - } else if (mark[1] == 0xDA) { + } else if (mark[1] == 0xDA) { /* SOS - start of scan */ /* start of scan image, lets not look behind of this */ ret = META_PARSING_DONE; jpeg_data->state = JPEG_PARSE_DONE; @@ -428,30 +423,34 @@ metadataparse_jpeg_reading (JpegParseData * jpeg_data, guint8 ** buf, chunk_size = READ (*buf, *bufsize) << 8; chunk_size += READ (*buf, *bufsize); - if (mark[1] == 0xE0) { /* may be JFIF */ + if (mark[1] == 0xE0) { /* APP0 - may be JFIF */ - if (chunk_size >= 16) { + /* FIXME: whats the 14 ? according to + * http://en.wikipedia.org/wiki/JFIF#JFIF_segment_format + * its the jfif segment without thumbnails + */ + if (chunk_size >= 14 + 2) { if (*bufsize < 14) { *next_size = (*buf - *next_start) + 14; ret = META_PARSING_NEED_MORE_DATA; goto done; } - if (0 == memcmp (JfifHeader, *buf, 5)) { + if (0 == memcmp (JfifHeader, *buf, sizeof (JfifHeader))) { jpeg_data->jfif_found = TRUE; } } - } else if (mark[1] == 0xE1) { /* may be it is Exif or XMP */ + } else if (mark[1] == 0xE1) { /* APP1 - may be it is Exif or XMP */ - if (chunk_size >= 8) { /* size2 'EXIF' 0x00 0x00 */ - if (*bufsize < 6) { - *next_size = (*buf - *next_start) + 6; + if (chunk_size >= sizeof (EXIF_HEADER) + 2) { + if (*bufsize < sizeof (EXIF_HEADER)) { + *next_size = (*buf - *next_start) + sizeof (EXIF_HEADER); ret = META_PARSING_NEED_MORE_DATA; goto done; } - if (0 == memcmp (ExifHeader, *buf, 6)) { + if (0 == memcmp (EXIF_HEADER, *buf, sizeof (EXIF_HEADER))) { MetadataChunk chunk; if (!jpeg_data->parse_only) { @@ -502,14 +501,14 @@ metadataparse_jpeg_reading (JpegParseData * jpeg_data, guint8 ** buf, goto done; } } - if (chunk_size >= 31) { /* size2 "http://ns.adobe.com/xap/1.0/" */ - if (*bufsize < 29) { - *next_size = (*buf - *next_start) + 29; + if (chunk_size >= sizeof (XMP_HEADER) + 2) { + if (*bufsize < sizeof (XMP_HEADER)) { + *next_size = (*buf - *next_start) + sizeof (XMP_HEADER); ret = META_PARSING_NEED_MORE_DATA; goto done; } - if (0 == memcmp (XmpHeader, *buf, 29)) { + if (0 == memcmp (XMP_HEADER, *buf, sizeof (XMP_HEADER))) { if (!jpeg_data->parse_only) { @@ -527,9 +526,9 @@ metadataparse_jpeg_reading (JpegParseData * jpeg_data, guint8 ** buf, /* if adapter has been provided, prepare to hold chunk */ if (jpeg_data->xmp_adapter) { - *buf += 29; - *bufsize -= 29; - jpeg_data->read = chunk_size - 2 - 29; + *buf += sizeof (XMP_HEADER); + *bufsize -= sizeof (XMP_HEADER); + jpeg_data->read = chunk_size - 2 - sizeof (XMP_HEADER); jpeg_data->state = JPEG_PARSE_XMP; ret = META_PARSING_DONE; goto done; @@ -550,8 +549,7 @@ metadataparse_jpeg_reading (JpegParseData * jpeg_data, guint8 ** buf, goto done; } - - if (0 == memcmp (PhotoshopHeader, *buf, 14)) { + if (0 == memcmp (PHOTOSHOP_HEADER, *buf, sizeof (PHOTOSHOP_HEADER))) { if (!jpeg_data->parse_only) { diff --git a/ext/metadata/metadataparsepng.c b/ext/metadata/metadataparsepng.c index 520ef20462..45c8da4b9f 100644 --- a/ext/metadata/metadataparsepng.c +++ b/ext/metadata/metadataparsepng.c @@ -363,6 +363,11 @@ metadataparse_png_reading (PngParseData * png_data, guint8 ** buf, mark[2] = READ (*buf, *bufsize); mark[3] = READ (*buf, *bufsize); + /* FIXME: use FOURCECC macros */ + + GST_DEBUG ("parsing png : 0x%02x%02x%02x%02x", + mark[0], mark[1], mark[2], mark[3]); + if (mark[0] == 'I' && mark[1] == 'E' && mark[2] == 'N' && mark[3] == 'D') { ret = META_PARSING_DONE; png_data->state = PNG_PARSE_DONE; diff --git a/ext/metadata/metadataxmp.h b/ext/metadata/metadataxmp.h index 05d89dac7c..47e4f11613 100644 --- a/ext/metadata/metadataxmp.h +++ b/ext/metadata/metadataxmp.h @@ -50,6 +50,11 @@ G_BEGIN_DECLS +/* + * defines + */ +#define XMP_HEADER "http://ns.adobe.com/xap/1.0/" + /* * external function prototypes */