Version: 0.8.2 beta1 & 0.9svn (using KDE KDE 3.4.2) Installed from: Mandriva RPMs Compiler: Default Mandriva 2006.0 compiler OS: Linux From cameraUI dialog (with USB mass storage), I select 2 pictures (I will include them) and ask them to be renamed with date and time then transfer them. As the generated name is the same for both picture, a dialog opens to rename the 2nd picture, but it shows only the first one. There is a clickable message saying that the picture is not on the local machine and offering to transfer it, but it does not work. We can't really take an action without seeing both files. (it seems also that the date&time used in the renaming where the one for the file, not the exif one, but that's another bug).
Created attachment 15827 [details] CameraUI dialog before starting the transfer
Created attachment 15828 [details] Rename Dialog
Created attachment 15829 [details] first file transfered (bluried and reduced)
Created attachment 15830 [details] 2nd file transfered (bluried and reduced)
SVN commit 576202 by cgilles: digikam from trunk: With UMS camera, fix full path of camera item for krename dialog to render preview properlly. Note: with gphoto2 camera, the problem still exist because camera items aren't in the local file system. This require a lots of advanced changes in the implementation. CCBUGS: 126427 M +7 -4 cameracontroller.cpp --- trunk/extragear/graphics/digikam/utilities/cameragui/cameracontroller.cpp #576201:576202 @@ -463,9 +463,9 @@ } case(CameraCommand::gp_open): { - QString folder = cmd->map["folder"].asString(); - QString file = cmd->map["file"].asString(); - QString dest = cmd->map["dest"].asString(); + QString folder = cmd->map["folder"].asString(); + QString file = cmd->map["file"].asString(); + QString dest = cmd->map["dest"].asString(); sendInfo(i18n("Retrieving file %1 from camera...").arg(file)); @@ -1138,7 +1138,10 @@ break; } - KIO::RenameDlg dlg(d->parent, i18n("Rename File"), file, dest, + // FIXME : see B.K.O #126427: with Gphoto camera, the cmaera file is not + // mounted in local and connot be display like a preview in dialog. + + KIO::RenameDlg dlg(d->parent, i18n("Rename File"), folder + QString("/") + file, dest, KIO::RenameDlg_Mode(KIO::M_MULTI | KIO::M_OVERWRITE | KIO::M_SKIP)); int result = dlg.exec();
Created attachment 18381 [details] digikam patch I suggest this patch. Note that without it the preview doesn't work for me even with mass storage camera.
Lubos, does your patch address the issue of the original bug-report, or does it relate to something else? Thanks in advance, Arnd
Arnd, The patch proposed cannot be applied, because picture downloading is done outside multithreading and it broke all To solve this problem sound like more complicated. In fact the picture must be downloaded of course in temp file before to check if a same file exist in target folder... to be able to show a preview. But this must be done to use the existing framework of camera interface. In fact more code need to be changed. I will review this problem during KDE4 port. Gilles
SVN commit 708206 by cgilles: digiKam from KDE3 branch : Camera Gui : always download the current picture as temp file before to check if target file already exist in target album. The advantage of this way is simple. We can always show a preview of picture from camera because file is in local file system. Also, this way work with all camera type (gphoto2 include). Important remarks : The Remane dialog is provided by KDE api (KIO::RenameDlg class). - With KDE 3.4.x, this dialog is not able to display preview of pictures properlly. - With KDE 3.5, the dialog have been revisted, work better, but sometime the preview doesn't work properlly with JPEG, and PNG files. It's never work with RAW file. This is duing of limitation in picture preview implementation provided by KDE API. I have not yet tested with KDE4. Possible solution of this problem is to use digiKam core to render preview. But we must fork KIO::RenameDlg for that, which is a wrong way. I recommend to post a new report in KDElibs to improve definitivly the pictures preview implementation to work properlly in all cases. M +104 -104 cameracontroller.cpp --- branches/extragear/kde3/graphics/digikam/utilities/cameragui/cameracontroller.cpp #708205:708206 @@ -375,6 +375,7 @@ KURL tempURL(dest); tempURL = tempURL.upURL(); tempURL.addPath( QString(".digikam-camera-tmp1-%1").arg(getpid())); + QString temp = tempURL.path(); bool result = d->camera->downloadItem(folder, file, tempURL.path()); @@ -406,7 +407,7 @@ // Convert Jpeg file to lossless format if necessary, // and move converted image to destination. - + if (convertJpeg && isJpegImage(tempURL.path())) { sendInfo(i18n("Converting %1 to lossless file format...").arg(file)); @@ -414,36 +415,16 @@ KURL tempURL2(dest); tempURL2 = tempURL2.upURL(); tempURL2.addPath( QString(".digikam-camera-tmp2-%1").arg(getpid())); + temp = tempURL2.path(); if (!jpegConvert(tempURL.path(), tempURL2.path(), file, losslessFormat)) { // convert failed. delete the temp file + unlink(QFile::encodeName(tempURL.path())); unlink(QFile::encodeName(tempURL2.path())); result = false; } - else - { - // move the file to the destination file - if (rename(QFile::encodeName(tempURL2.path()), QFile::encodeName(dest)) != 0) - { - // rename failed. delete the temp file - unlink(QFile::encodeName(tempURL2.path())); - result = false; - } - } - - unlink(QFile::encodeName(tempURL.path())); } - else - { - // move the file to the destination file - if (rename(QFile::encodeName(tempURL.path()), QFile::encodeName(dest)) != 0) - { - // rename failed. delete the temp file - unlink(QFile::encodeName(tempURL.path())); - result = false; - } - } } if (result) @@ -452,6 +433,7 @@ event->map.insert("folder", QVariant(folder)); event->map.insert("file", QVariant(file)); event->map.insert("dest", QVariant(dest)); + event->map.insert("temp", QVariant(temp)); QApplication::postEvent(parent, event); } else @@ -922,7 +904,104 @@ { QString folder = QDeepCopy<QString>(event->map["folder"].asString()); QString file = QDeepCopy<QString>(event->map["file"].asString()); - emit signalDownloaded(folder, file, GPItemInfo::DownloadedYes); + QString dest = QDeepCopy<QString>(event->map["dest"].asString()); + QString temp = QDeepCopy<QString>(event->map["temp"].asString()); + + d->timer->stop(); + + bool skip = false; + bool cancel = false; + bool overwrite = false; + + // Check if dest file already exist. + + if (!d->overwriteAll) + { + struct stat info; + + while (::stat(QFile::encodeName(dest), &info) == 0) + { + if (d->skipAll) + { + skip = true; + break; + } + + KIO::RenameDlg dlg(d->parent, i18n("Rename File"), + folder + QString("/") + file, dest, + KIO::RenameDlg_Mode(KIO::M_MULTI | KIO::M_OVERWRITE | KIO::M_SKIP)); + + int result = dlg.exec(); + dest = dlg.newDestURL().path(); + + switch (result) + { + case KIO::R_CANCEL: + { + cancel = true; + break; + } + case KIO::R_SKIP: + { + skip = true; + break; + } + case KIO::R_AUTO_SKIP: + { + d->skipAll = true; + skip = true; + break; + } + case KIO::R_OVERWRITE: + { + overwrite = true; + break; + } + case KIO::R_OVERWRITE_ALL: + { + d->overwriteAll = true; + overwrite = true; + break; + } + default: + break; + } + + if (cancel || skip || overwrite) + break; + } + } + + if (cancel) + { + unlink(QFile::encodeName(temp)); + slotCancel(); + d->timer->start(50); + emit signalSkipped(folder, file); + return; + } + else if (skip) + { + unlink(QFile::encodeName(temp)); + d->timer->start(50); + emit signalInfoMsg(i18n("Skipped file %1").arg(file)); + emit signalSkipped(folder, file); + return; + } + + // move the file to the destination file + if (rename(QFile::encodeName(temp), QFile::encodeName(dest)) != 0) + { + // rename failed. delete the temp file + unlink(QFile::encodeName(temp)); + d->timer->start(50); + emit signalDownloaded(folder, file, GPItemInfo::DownloadFailed); + } + else + { + d->timer->start(50); + emit signalDownloaded(folder, file, GPItemInfo::DownloadedYes); + } break; } case (CameraEvent::gp_downloadFailed) : @@ -1102,10 +1181,6 @@ CameraCommand* cmd = d->cmdQueue.head(); - bool skip = false; - bool cancel = false; - bool overwrite = false; - QString folder; QString file; QString dest; @@ -1128,84 +1203,9 @@ folder = QDeepCopy<QString>(cmd->map["folder"].asString()); file = QDeepCopy<QString>(cmd->map["file"].asString()); dest = QDeepCopy<QString>(cmd->map["dest"].asString()); - - if (!d->overwriteAll) - { - struct stat info; - - while (::stat(QFile::encodeName(dest), &info) == 0) - { - if (d->skipAll) - { - skip = true; - break; - } - - // FIXME : see B.K.O #126427: with Gphoto camera, the camera folder is not - // mounted in local and camera picture cannot be display like a preview in dialog. - - KIO::RenameDlg dlg(d->parent, i18n("Rename File"), folder + QString("/") + file, dest, - KIO::RenameDlg_Mode(KIO::M_MULTI | KIO::M_OVERWRITE | KIO::M_SKIP)); - - int result = dlg.exec(); - dest = dlg.newDestURL().path(); - - switch (result) - { - case KIO::R_CANCEL: - { - cancel = true; - break; - } - case KIO::R_SKIP: - { - skip = true; - break; - } - case KIO::R_AUTO_SKIP: - { - d->skipAll = true; - skip = true; - break; - } - case KIO::R_OVERWRITE: - { - overwrite = true; - break; - } - case KIO::R_OVERWRITE_ALL: - { - d->overwriteAll = true; - overwrite = true; - break; - } - default: - break; - } - - if (cancel || skip || overwrite) - break; - } - } - - cmd->map["dest"] = QVariant(QDeepCopy<QString>(dest)); + cmd->map["dest"] = QVariant(QDeepCopy<QString>(dest)); } - if (cancel) - { - slotCancel(); - d->timer->start(50, false); - return; - } - else if (skip) - { - d->cmdQueue.dequeue(); - emit signalInfoMsg(i18n("Skipped file %1").arg(file)); - emit signalSkipped(folder, file); - d->timer->start(50, false); - return; - } - d->thread->start(); d->timer->start(50, false); }
SVN commit 708227 by cgilles: digiKam from trunk (KDE4): backport commits #708206 from KDE3 branch BUG: 126427 M +97 -119 cameracontroller.cpp M +5 -2 cameracontroller.h --- trunk/extragear/graphics/digikam/utilities/cameragui/cameracontroller.cpp #708226:708227 @@ -23,15 +23,6 @@ * * ============================================================ */ -// C Ansi includes. - -extern "C" -{ -#include <sys/types.h> -#include <sys/stat.h> -#include <unistd.h> -} - // C++ includes. #include <typeinfo> @@ -94,38 +85,21 @@ QMap<QString,QVariant> map; }; -class RenameResult -{ -public: - - RenameResult() - { - skip = false; - overwrite = false; - cancel = false; - } - - bool skip; - bool overwrite; - bool cancel; - QString dest; -}; - class CameraControllerPriv { public: CameraControllerPriv() { - close = false; - overwriteAll = false; - skipAll = false; - canceled = false; - running = false; + close = false; + overwriteAll = false; + skipAll = false; + canceled = false; + running = false; downloadTotal = 0; - parent = 0; - timer = 0; - camera = 0; + parent = 0; + timer = 0; + camera = 0; } bool close; @@ -191,8 +165,8 @@ qRegisterMetaType<GPItemInfo>("GPItemInfo"); qRegisterMetaType<GPItemInfoList>("GPItemInfoList"); - connect(this, SIGNAL(signalInternalNeedRename(const QString&, const QString&, const QString&, RenameResult *)), - this, SLOT(slotNeedRename(const QString&, const QString&, const QString&, RenameResult *)), + connect(this, SIGNAL(signalInternalCheckRename(const QString&, const QString&, const QString&, const QString&)), + this, SLOT(slotCheckRename(const QString&, const QString&, const QString&, const QString&)), Qt::BlockingQueuedConnection); connect(this, SIGNAL(signalInternalDownloadFailed(const QString&, const QString&)), @@ -394,21 +368,6 @@ if (!d->overwriteAll) { - QFileInfo info(dest); - if (info.exists()) - { - if (!d->skipAll) - { - // do UI operation from main thread - RenameResult result; - emit signalInternalNeedRename(folder, file, dest, &result); - if (result.skip || result.cancel) - break; - if (result.overwrite) - dest = result.dest; - } - } - bool autoRotate = cmd->map["autoRotate"].toBool(); bool fixDateTime = cmd->map["fixDateTime"].toBool(); QDateTime newDateTime = cmd->map["newDateTime"].toDateTime(); @@ -430,6 +389,7 @@ KUrl tempURL(dest); tempURL = tempURL.upUrl(); tempURL.addPath(QString(".digikam-camera-tmp1-%1").arg(getpid())); + QString temp = tempURL.path(); bool result = d->camera->downloadItem(folder, file, tempURL.path()); @@ -469,49 +429,25 @@ KUrl tempURL2(dest); tempURL2 = tempURL2.upUrl(); tempURL2.addPath(QString(".digikam-camera-tmp2-%1").arg(getpid())); + temp = tempURL2.path(); if (!jpegConvert(tempURL.path(), tempURL2.path(), file, losslessFormat)) { // convert failed. delete the temp file + unlink(QFile::encodeName(tempURL.path())); unlink(QFile::encodeName(tempURL2.path())); result = false; } - else - { - // move the file to the destination file - if (rename(QFile::encodeName(tempURL2.path()), QFile::encodeName(dest)) != 0) - { - // rename failed. delete the temp file - unlink(QFile::encodeName(tempURL2.path())); - result = false; - } - } - - unlink(QFile::encodeName(tempURL.path())); } - else - { - // move the file to the destination file - if (rename(QFile::encodeName(tempURL.path()), QFile::encodeName(dest)) != 0) - { - // rename failed. delete the temp file - unlink(QFile::encodeName(tempURL.path())); - result = false; - } - } } - if (result) + if (!d->skipAll) { - emit signalDownloaded(folder, file, GPItemInfo::DownloadedYes); + // do UI operation from main thread + emit signalInternalCheckRename(folder, file, dest, temp); } - else - { - emit signalInternalDownloadFailed(folder, file); - emit signalDownloaded(folder, file, GPItemInfo::DownloadFailed); - } - break; } + break; } case(CameraCommand::gp_open): { @@ -620,57 +556,99 @@ emit signalInfoMsg(msg); } -void CameraController::slotNeedRename(const QString &folder, const QString &file, const QString &dest, RenameResult *renameResult) +void CameraController::slotCheckRename(const QString &folder, const QString &file, + const QString &destination, const QString &temp) { - // FIXME : see B.K.O #126427: with Gphoto camera, the camera folder is not - // mounted in local and camera picture cannot be display like a preview in dialog. + bool skip = false; + bool cancel = false; + bool overwrite = false; + QString dest = destination; - KIO::RenameDialog dlg(d->parent, i18n("Rename File"), folder + QString("/") + file, dest, - KIO::RenameDialog_Mode(KIO::M_MULTI | KIO::M_OVERWRITE | KIO::M_SKIP)); + // Check if dest file already exist. - int result = dlg.exec(); - renameResult->dest = dlg.newDestUrl().path(); - - switch (result) + if (!d->overwriteAll) { - case KIO::R_CANCEL: + QFileInfo info(dest); + + while (info.exists()) { - renameResult->cancel = true; - break; + if (d->skipAll) + { + skip = true; + break; + } + + KIO::RenameDialog dlg(d->parent, i18n("Rename File"), + folder + QString("/") + file, dest, + KIO::RenameDialog_Mode(KIO::M_MULTI | KIO::M_OVERWRITE | KIO::M_SKIP)); + + int result = dlg.exec(); + dest = dlg.newDestUrl().path(); + + switch (result) + { + case KIO::R_CANCEL: + { + cancel = true; + break; + } + case KIO::R_SKIP: + { + skip = true; + break; + } + case KIO::R_AUTO_SKIP: + { + d->skipAll = true; + skip = true; + break; + } + case KIO::R_OVERWRITE: + { + overwrite = true; + break; + } + case KIO::R_OVERWRITE_ALL: + { + d->overwriteAll = true; + overwrite = true; + break; + } + default: + break; + } + + if (cancel || skip || overwrite) + break; } - case KIO::R_SKIP: - { - renameResult->skip = true; - break; - } - case KIO::R_AUTO_SKIP: - { - d->skipAll = true; - renameResult->skip = true; - break; - } - case KIO::R_OVERWRITE: - { - renameResult->overwrite = true; - break; - } - case KIO::R_OVERWRITE_ALL: - { - d->overwriteAll = true; - renameResult->overwrite = true; - break; - } } - if (renameResult->cancel) + if (cancel) { + unlink(QFile::encodeName(temp)); slotCancel(); + emit signalSkipped(folder, file); + return; } - else if (renameResult->skip) + else if (skip) { - emit signalInfoMsg(i18n("Skipped file %1",file)); - emit signalSkipped(folder, file); + unlink(QFile::encodeName(temp)); + emit signalInfoMsg(i18n("Skipped file %1", file)); + emit signalSkipped(folder, file); + return; } + + // move the file to the destination file + if (rename(QFile::encodeName(temp), QFile::encodeName(dest)) != 0) + { + // rename failed. delete the temp file + unlink(QFile::encodeName(temp)); + emit signalDownloaded(folder, file, GPItemInfo::DownloadFailed); + } + else + { + emit signalDownloaded(folder, file, GPItemInfo::DownloadedYes); + } } void CameraController::slotDownloadFailed(const QString &folder, const QString &file) --- trunk/extragear/graphics/digikam/utilities/cameragui/cameracontroller.h #708226:708227 @@ -100,7 +100,9 @@ void executeCommand(CameraCommand *cmd); signals: - void signalInternalNeedRename(const QString &folder, const QString &file, const QString &dest, RenameResult *renameResult); + + void signalInternalCheckRename(const QString &folder, const QString &file, + const QString &destination, const QString &temp); void signalInternalDownloadFailed(const QString &folder, const QString &file); void signalInternalUploadFailed(const QString &folder, const QString &file, const QString &src); void signalInternalDeleteFailed(const QString &folder, const QString &file); @@ -109,7 +111,8 @@ private slots: - void slotNeedRename(const QString &folder, const QString &file, const QString &dest, RenameResult *renameResult); + void slotCheckRename(const QString &folder, const QString &file, + const QString &destination, const QString& temp); void slotDownloadFailed(const QString &folder, const QString &file); void slotUploadFailed(const QString &folder, const QString &file, const QString &src); void slotDeleteFailed(const QString &folder, const QString &file);