diff --git a/rtengine/iccjpeg.cc b/rtengine/iccjpeg.cc index 85721f631..7500d27ec 100644 --- a/rtengine/iccjpeg.cc +++ b/rtengine/iccjpeg.cc @@ -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 diff --git a/rtengine/imageio.cc b/rtengine/imageio.cc index 9dc8d8b6d..6d2ac881c 100644 --- a/rtengine/imageio.cc +++ b/rtengine/imageio.cc @@ -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: . 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); @@ -302,7 +302,15 @@ int ImageIO::loadPNG (Glib::ustring fname) { allocate (width, height); int rowlen = width*3*bit_depth/8; - unsigned char *row = new unsigned char [rowlen]; + 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;isetProgressStr ("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); @@ -825,20 +833,62 @@ int ImageIO::savePNG (Glib::ustring fname, int compression, volatile int bps) { return IMIO_SUCCESS; } + +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) { + + FILE *file = safe_g_fopen_WriteBinLock (fname); + if (!file) + return IMIO_CANNOTWRITEFILE; - jpeg_compress_struct cinfo; - jpeg_error_mgr jerr; - - cinfo.err = jpeg_std_error (&jerr); + 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; + } + jpeg_create_compress (&cinfo); - FILE *file = safe_g_fopen_WriteBinLock (fname); - if (!file) - return IMIO_CANNOTREADFILE; if (pl) { pl->setProgressStr ("PROGRESSBAR_SAVEJPEG"); @@ -887,13 +937,13 @@ int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) { unsigned char* buffer = new unsigned char[165535]; //FIXME: no buffer size check so it can be overflowed in createJPEGMarker() for large tags, and then software will crash unsigned int size; // assemble and write exif marker - if (exifRoot) { + if (exifRoot) { int size = rtexif::ExifManager::createJPEGMarker (exifRoot, exifChange, cinfo.image_width, cinfo.image_height, buffer); if (size>0 && size<65530) jpeg_write_marker(&cinfo, JPEG_APP0+1, buffer, size); } // assemble and write iptc marker - if (iptc) { + if (iptc) { unsigned char* iptcdata; bool error = false; if (iptc_data_save (iptc, &iptcdata, &size)) { @@ -909,7 +959,9 @@ 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); @@ -917,30 +969,44 @@ int ImageIO::saveJPEG (Glib::ustring fname, int quality, int subSamp) { // write image data 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); - fclose (file); - return IMIO_READERROR; + jpeg_destroy_compress (&cinfo); + delete [] row; + fclose (file); + safe_g_remove(fname); + return IMIO_CANNOTWRITEFILE; } if (pl && !(cinfo.next_scanline%100)) pl->setProgress ((double)(cinfo.next_scanline)/cinfo.image_height); } - jpeg_finish_compress (&cinfo); + jpeg_finish_compress (&cinfo); 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,7 +1073,9 @@ 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); } else { @@ -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); } - return IMIO_SUCCESS; + if(writeOk) + return IMIO_SUCCESS; + else { + safe_g_remove(fname); + return IMIO_CANNOTWRITEFILE; + } } // PNG read and write routines: diff --git a/rtengine/imageio.h b/rtengine/imageio.h index 81b64fd8a..d50c857bf 100644 --- a/rtengine/imageio.h +++ b/rtengine/imageio.h @@ -26,6 +26,7 @@ #define IMIO_READERROR 4 #define IMIO_VARIANTNOTSUPPORTED 5 #define IMIO_FILETYPENOTSUPPORTED 6 +#define IMIO_CANNOTWRITEFILE 7 #include "rtengine.h" #include diff --git a/rtgui/batchqueue.cc b/rtgui/batchqueue.cc index b6370ffaf..6f4bd10f9 100644 --- a/rtgui/batchqueue.cc +++ b/rtgui/batchqueue.cc @@ -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,13 +627,17 @@ 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()); @@ -654,13 +659,14 @@ rtengine::ProcessingJob* BatchQueue::imageReady (rtengine::IImage16* img) { processing->selected = false; } // remove button set - { - // ButtonSet have Cairo::Surface which might be rendered while we're trying to delete them - GThreadLock lock; - next->removeButtonSet (); - } + 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; + processing->removeButtonSet (); + } if (saveBatchQueue( )) { safe_g_remove( processedParams ); // Delete all files in directory \batch when finished, just to be sure to remove zombies diff --git a/rtgui/editorpanel.cc b/rtgui/editorpanel.cc index 27454f500..6cdb2c8c1 100755 --- a/rtgui/editorpanel.cc +++ b/rtgui/editorpanel.cc @@ -224,8 +224,8 @@ EditorPanel::EditorPanel (FilePanel* filePanel) // Zoom panel iops->pack_end (*iareapanel->imageArea->zoomPanel, Gtk::PACK_SHRINK, 1); iops->pack_end (*vsepz3, Gtk::PACK_SHRINK, 2); - - navPrev = navNext = navSync = NULL; + + navPrev = navNext = navSync = NULL; if (!simpleEditor && !options.tabbedUI){ // Navigation buttons Gtk::Image *navPrevImage = Gtk::manage (new RTImage ("nav-prev.png")); @@ -326,15 +326,15 @@ EditorPanel::EditorPanel (FilePanel* filePanel) tbRightPanel_1->signal_toggled().connect( sigc::mem_fun(*this, &EditorPanel::tbRightPanel_1_toggled) ); saveimgas->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::saveAsPressed) ); queueimg->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::queueImgPressed) ); - sendtogimp->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::sendToGimpPressed) ); - if(navPrev) + sendtogimp->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::sendToGimpPressed) ); + if(navPrev) navPrev->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::openPreviousEditorImage) ); - if(navNext) + if(navNext) navNext->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::openNextEditorImage) ); - if(navSync) + if(navSync) navSync->signal_pressed().connect( sigc::mem_fun(*this, &EditorPanel::syncFileBrowser) ); ShowHideSidePanelsconn = tbShowHideSidePanels->signal_toggled().connect ( sigc::mem_fun(*this, &EditorPanel::toggleSidePanels), true); - if (tbTopPanel_1) + if (tbTopPanel_1) tbTopPanel_1->signal_toggled().connect( sigc::mem_fun(*this, &EditorPanel::tbTopPanel_1_toggled) ); } @@ -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 *pc,rtengine::IImage16* } } else { Glib::ustring msg_ = Glib::ustring("") + fname + ": Error during image saving\n"; - Gtk::MessageDialog msgd (*parent, msg_, true, Gtk::MESSAGE_ERROR, Gtk::BUTTONS_OK, true); - msgd.run (); + error(M("MAIN_MSG_CANNOTSAVE"), ""+fname+""); } saveimgas->set_sensitive(true); diff --git a/rtgui/editorpanel.h b/rtgui/editorpanel.h index d038c0dd2..38154cd73 100644 --- a/rtgui/editorpanel.h +++ b/rtgui/editorpanel.h @@ -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