Stop the queue if error saving, Issue 2257 and Segfault on indexed PNG, Issue 2556

This commit is contained in:
Ingo
2014-11-28 00:09:55 +01:00
parent fb6861e481
commit 5980774a76
6 changed files with 154 additions and 68 deletions

View File

@@ -70,8 +70,7 @@ write_icc_profile (j_compress_ptr cinfo,
icc_data_len -= length;
/* Write the JPEG marker header (APP2 code and marker length) */
jpeg_write_m_header(cinfo, ICC_MARKER,
(unsigned int) (length + ICC_OVERHEAD_LEN));
jpeg_write_m_header(cinfo, ICC_MARKER, (unsigned int) (length + ICC_OVERHEAD_LEN));
/* Write the marker identifying string "ICC_PROFILE" (null-terminated).
* We code it in this less-than-transparent way so that the code works

View File

@@ -267,21 +267,21 @@ int ImageIO::loadPNG (Glib::ustring fname) {
//retrieving image information
png_uint_32 width,height;
int bit_depth,color_type,interlace_type,compression_type,filter_method;
png_get_IHDR(png,info,&width,&height,&bit_depth,&color_type,&interlace_type,&compression_type, &filter_method);
png_get_IHDR(png, info, &width, &height, &bit_depth, &color_type, &interlace_type, &compression_type, &filter_method);
//converting to 32bpp format
if (color_type==PNG_COLOR_TYPE_PALETTE) png_set_palette_to_rgb(png);
if (color_type==PNG_COLOR_TYPE_PALETTE || interlace_type!=PNG_INTERLACE_NONE ) {
// we don't support interlaced png or png with palette
png_destroy_read_struct (&png, &info, &end_info);
fclose (file);
printf("%s uses an unsupported feature: <palette-indexed colors|interlacing>. Skipping.\n",fname.data());
return IMIO_VARIANTNOTSUPPORTED;
}
if (color_type==PNG_COLOR_TYPE_GRAY || color_type==PNG_COLOR_TYPE_GRAY_ALPHA)
png_set_gray_to_rgb(png);
if (png_get_valid(png,info,PNG_INFO_tRNS)) png_set_tRNS_to_alpha(png);
if (interlace_type!=PNG_INTERLACE_NONE) {
png_destroy_read_struct (&png, &info, &end_info);
fclose (file);
return IMIO_VARIANTNOTSUPPORTED;
}
if (png_get_valid(png,info,PNG_INFO_tRNS))
png_set_tRNS_to_alpha(png);
if (color_type & PNG_COLOR_MASK_ALPHA)
png_set_strip_alpha(png);
@@ -304,6 +304,14 @@ int ImageIO::loadPNG (Glib::ustring fname) {
int rowlen = width*3*bit_depth/8;
unsigned char *row = new unsigned char [rowlen];
// set a new jump point to avoid memory leak
if (setjmp (png_jmpbuf(png))) {
png_destroy_read_struct (&png, &info, &end_info);
fclose (file);
delete [] row;
return IMIO_READERROR;
}
for (unsigned int i=0;i<height;i++) {
png_read_row (png, (png_byte*)row, NULL);
@@ -753,7 +761,7 @@ int ImageIO::savePNG (Glib::ustring fname, int compression, volatile int bps) {
FILE *file = safe_g_fopen_WriteBinLock (fname);
if (!file)
return IMIO_CANNOTREADFILE;
return IMIO_CANNOTWRITEFILE;
if (pl) {
pl->setProgressStr ("PROGRESSBAR_SAVEPNG");
@@ -775,7 +783,7 @@ int ImageIO::savePNG (Glib::ustring fname, int compression, volatile int bps) {
if (setjmp(png_jmpbuf(png))) {
png_destroy_write_struct (&png,&info);
fclose(file);
return IMIO_READERROR;
return IMIO_CANNOTWRITEFILE;
}
png_set_write_fn (png, file, png_write_data, png_flush);
@@ -826,19 +834,61 @@ int ImageIO::savePNG (Glib::ustring fname, int compression, volatile int bps) {
}
typedef struct {
struct jpeg_error_mgr pub; /* "public" fields */
jmp_buf setjmp_buffer; /* for return to caller */
} my_error_mgr;
void my_error_exit (j_common_ptr cinfo) {
/* cinfo->err really points to a my_error_mgr struct, so coerce pointer */
my_error_mgr *myerr = (my_error_mgr*) cinfo->err;
/* Always display the message. */
/* We could postpone this until after returning, if we chose. */
(*cinfo->err->output_message) (cinfo);
/* Return control to the setjmp point */
#if defined( WIN32 ) && defined( __x86_64__ )
__builtin_longjmp(myerr->setjmp_buffer, 1);
#else
longjmp(myerr->setjmp_buffer, 1);
#endif
}
// Quality 0..100, subsampling: 1=low quality, 2=medium, 3=high
int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) {
jpeg_compress_struct cinfo;
jpeg_error_mgr jerr;
FILE *file = safe_g_fopen_WriteBinLock (fname);
if (!file)
return IMIO_CANNOTWRITEFILE;
jpeg_compress_struct cinfo;
/* We use our private extension JPEG error handler.
Note that this struct must live as long as the main JPEG parameter
struct, to avoid dangling-pointer problems.
*/
my_error_mgr jerr;
/* We set up the normal JPEG error routines, then override error_exit. */
cinfo.err = jpeg_std_error(&jerr.pub);
jerr.pub.error_exit = my_error_exit;
/* Establish the setjmp return context for my_error_exit to use. */
#if defined( WIN32 ) && defined( __x86_64__ )
if (__builtin_setjmp(jerr.setjmp_buffer)) {
#else
if (setjmp(jerr.setjmp_buffer)) {
#endif
/* If we get here, the JPEG code has signaled an error.
We need to clean up the JPEG object, close the file, remove the already saved part of the file and return.
*/
jpeg_destroy_compress(&cinfo);
fclose(file);
safe_g_remove(fname);
return IMIO_CANNOTWRITEFILE;
}
cinfo.err = jpeg_std_error (&jerr);
jpeg_create_compress (&cinfo);
FILE *file = safe_g_fopen_WriteBinLock (fname);
if (!file)
return IMIO_CANNOTREADFILE;
if (pl) {
pl->setProgressStr ("PROGRESSBAR_SAVEJPEG");
@@ -910,6 +960,8 @@ int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) {
if (!error)
jpeg_write_marker(&cinfo, JPEG_APP0+13, buffer, bytes);
}
delete [] buffer;
// write icc profile to the output
if (profileData)
write_icc_profile (&cinfo, (JOCTET*)profileData, profileLength);
@@ -918,15 +970,31 @@ int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) {
int rowlen = width*3;
unsigned char *row = new unsigned char [rowlen];
/* To avoid memory leaks we establish a new setjmp return context for my_error_exit to use. */
#if defined( WIN32 ) && defined( __x86_64__ )
if (__builtin_setjmp(jerr.setjmp_buffer)) {
#else
if (setjmp(jerr.setjmp_buffer)) {
#endif
/* If we get here, the JPEG code has signaled an error.
We need to clean up the JPEG object, close the file, remove the already saved part of the file and return.
*/
delete [] row;
jpeg_destroy_compress(&cinfo);
fclose(file);
safe_g_remove(fname);
return IMIO_CANNOTWRITEFILE;
}
while (cinfo.next_scanline < cinfo.image_height) {
getScanline (cinfo.next_scanline, row, 8);
if (jpeg_write_scanlines (&cinfo, &row, 1) < 1) {
jpeg_finish_compress (&cinfo);
jpeg_destroy_compress (&cinfo);
delete [] row;
fclose (file);
return IMIO_READERROR;
safe_g_remove(fname);
return IMIO_CANNOTWRITEFILE;
}
if (pl && !(cinfo.next_scanline%100))
@@ -937,10 +1005,8 @@ int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) {
jpeg_destroy_compress (&cinfo);
delete [] row;
delete [] buffer;
fclose (file);
if (pl) {
pl->setProgressStr ("PROGRESSBAR_READY");
pl->setProgress (1.0);
@@ -952,7 +1018,7 @@ int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) {
int ImageIO::saveTIFF (Glib::ustring fname, int bps, bool uncompressed) {
//TODO: Handling 32 bits floating point output images!
bool writeOk = true;
int width = getW ();
int height = getH ();
@@ -967,7 +1033,7 @@ int ImageIO::saveTIFF (Glib::ustring fname, int bps, bool uncompressed) {
if (!file) {
delete [] linebuffer;
return IMIO_CANNOTREADFILE;
return IMIO_CANNOTWRITEFILE;
}
if (pl) {
@@ -1007,6 +1073,8 @@ int ImageIO::saveTIFF (Glib::ustring fname, int bps, bool uncompressed) {
pl->setProgress ((double)(i+1)/height);
}
delete [] buffer;
if (ferror(file))
writeOk = false;
fclose (file);
}
@@ -1022,7 +1090,7 @@ int ImageIO::saveTIFF (Glib::ustring fname, int bps, bool uncompressed) {
#endif
if (!out) {
delete [] linebuffer;
return IMIO_CANNOTREADFILE;
return IMIO_CANNOTWRITEFILE;
}
if (pl) {
@@ -1089,11 +1157,14 @@ int ImageIO::saveTIFF (Glib::ustring fname, int bps, bool uncompressed) {
if (TIFFWriteScanline (out, linebuffer, row, 0) < 0) {
TIFFClose (out);
delete [] linebuffer;
return IMIO_READERROR;
return IMIO_CANNOTWRITEFILE;
}
if (pl && !(row%100))
pl->setProgress ((double)(row+1)/height);
}
if (TIFFFlush(out)!=1)
writeOk = false;
TIFFClose (out);
}
@@ -1103,7 +1174,12 @@ int ImageIO::saveTIFF (Glib::ustring fname, int bps, bool uncompressed) {
pl->setProgress (1.0);
}
if(writeOk)
return IMIO_SUCCESS;
else {
safe_g_remove(fname);
return IMIO_CANNOTWRITEFILE;
}
}
// PNG read and write routines:

View File

@@ -26,6 +26,7 @@
#define IMIO_READERROR 4
#define IMIO_VARIANTNOTSUPPORTED 5
#define IMIO_FILETYPENOTSUPPORTED 6
#define IMIO_CANNOTWRITEFILE 7
#include "rtengine.h"
#include <glibmm.h>

View File

@@ -608,7 +608,8 @@ rtengine::ProcessingJob* BatchQueue::imageReady (rtengine::IImage16* img) {
err = img->saveAsJPEG (fname, saveFormat.jpegQuality, saveFormat.jpegSubSamp);
img->free ();
if (err) throw Glib::FileError(Glib::FileError::FAILED, M("MAIN_MSG_CANNOTSAVE"));
if (err)
throw Glib::FileError(Glib::FileError::FAILED, M("MAIN_MSG_CANNOTSAVE")+"\n"+fname);
if (saveFormat.saveParams) {
// We keep the extension to avoid overwriting the profile when we have
@@ -626,14 +627,18 @@ rtengine::ProcessingJob* BatchQueue::imageReady (rtengine::IImage16* img) {
Glib::ustring processedParams = processing->savedParamsFile;
// delete from the queue
delete processing; processing = NULL;
bool queueEmptied=false;
bool remove_button_set = false;
{
// TODO: Check for Linux
#if PROTECT_VECTORS
MYWRITERLOCK(l, entryRW);
#endif
delete processing;
processing = NULL;
fd.erase (fd.begin());
// return next job
@@ -654,12 +659,13 @@ rtengine::ProcessingJob* BatchQueue::imageReady (rtengine::IImage16* img) {
processing->selected = false;
}
// remove button set
{
remove_button_set = true;
}
}
if (remove_button_set) {
// ButtonSet have Cairo::Surface which might be rendered while we're trying to delete them
GThreadLock lock;
next->removeButtonSet ();
}
}
processing->removeButtonSet ();
}
if (saveBatchQueue( )) {
safe_g_remove( processedParams );

View File

@@ -699,17 +699,21 @@ void EditorPanel::refreshProcessingState (bool inProcessingP) {
struct errparams {
Glib::ustring descr;
Glib::ustring title;
EditorPanelIdleHelper* epih;
};
void EditorPanel::displayError (Glib::ustring descr) {
if (parent) {
Gtk::MessageDialog* msgd = new Gtk::MessageDialog (*parent, descr, true, Gtk::MESSAGE_ERROR, Gtk::BUTTONS_OK, true);
msgd->set_title (M("MAIN_MSG_CANNOTSAVE"));
msgd->run ();
delete msgd;
}
void EditorPanel::displayError (Glib::ustring title, Glib::ustring descr) {
GtkWidget* msgd = gtk_message_dialog_new_with_markup (NULL,
GTK_DIALOG_DESTROY_WITH_PARENT,
GTK_MESSAGE_ERROR,
GTK_BUTTONS_OK,
descr.data());
gtk_window_set_title((GtkWindow*)msgd, title.data());
g_signal_connect_swapped (msgd, "response",
G_CALLBACK (gtk_widget_destroy),
msgd);
gtk_widget_show_all (msgd);
}
int disperrorUI (void* data) {
@@ -725,18 +729,19 @@ int disperrorUI (void* data) {
return 0;
}
p->epih->epanel->displayError (p->descr);
p->epih->epanel->displayError (p->title, p->descr);
p->epih->pending--;
delete p;
return 0;
}
void EditorPanel::error (Glib::ustring descr) {
void EditorPanel::error (Glib::ustring title, Glib::ustring descr) {
epih->pending++;
errparams* p = new errparams;
p->descr = descr;
p->title = title;
p->epih = epih;
g_idle_add (disperrorUI, p);
}
@@ -1129,8 +1134,7 @@ bool EditorPanel::idle_imageSaved(ProgressConnector<int> *pc,rtengine::IImage16*
}
} else {
Glib::ustring msg_ = Glib::ustring("<b>") + fname + ": Error during image saving\n</b>";
Gtk::MessageDialog msgd (*parent, msg_, true, Gtk::MESSAGE_ERROR, Gtk::BUTTONS_OK, true);
msgd.run ();
error(M("MAIN_MSG_CANNOTSAVE"), "<b>"+fname+"</b>");
}
saveimgas->set_sensitive(true);

View File

@@ -153,8 +153,8 @@ class EditorPanel : public Gtk::VBox,
void setProgress (double p);
void setProgressStr (Glib::ustring str);
void setProgressState (bool inProcessing);
void error (Glib::ustring descr);
void displayError (Glib::ustring descr); // this is called by error in the gtk thread
void error (Glib::ustring title, Glib::ustring descr);
void displayError (Glib::ustring title, Glib::ustring descr); // this is called by error in the gtk thread
void refreshProcessingState (bool inProcessing); // this is called by setProcessingState in the gtk thread
// PParamsChangeListener interface