Bug 82843

I agree. this is a waste of time.

Several wishes are related to this bug:
- Bug 59602, multiple copies of a cd
- Bug 77312, function to compare (verify) project with CD/DVD content
- Bug 77432, option to save a .md5sum file on every DataCD/DVD

Currently, the k3b behavior is to compute the image md5sum when loading the image and again each time the md5sum is needed. This is quite annoying -- with the fact the tray is opened between the burning and the check -- especially for multiple copies.

I think the behavior should be to compute the md5sum only once when the image is loaded or created. If a .iso.md5sum file with the same name exists, it should be used to verify the download. The md5sum should then be stored with the possibility to save it for later verifications. If a verification as been requested for after the burning, the stored md5sum should be used.

*** This bug has been confirmed by popular vote. ***

Created attachment 12917 [details]
Patch to fix bug 82843

I created this patch that appears to fix this bug.  Please let me know the
results if you test it.

Comment on attachment 12917 [details]
Patch to fix bug 82843

Better patch submitted

Created attachment 12918 [details]
Patch to fix bug 82843

Thsi is a better patch that the one I submitted just before.  I made the
corresponding changes for burning DVDs as well.

*** Bug 88841 has been marked as a duplicate of this bug. ***

Just to add to comment 2 ... you talk about ".iso.md5sum" - I think that ".md5" (with and without ".iso" part) is much more common than ".md5sum" ... but the program should support all well-known naming schemes and, if possible, other checksuming formats as well (see bug 112270).

I think checking md5sums should be optional. I never need to check the sums and that's why I can't use K3B to burn images. I just takes too much time and stresses my hard drive.

nonesense. Just click the start button and k3b will cancel the md5sum 
calculation.

*** Bug 125544 has been marked as a duplicate of this bug. ***

Sebastian, sure, the /initial/ md5 calculation can be cancelled. But when enabling "Verify", the md5sum is /again/ calculated for /every/ copy of the disk.
So, for making five copies of an .iso image with verify, the .iso file is being read ten times - five times for the burn, and another five times for calculating the md5sum.

k3b should /remember/ the md5sum of the image it is currently using, that is all.

SVN commit 597651 by trueg:

Use the ChecksumPipe to calculate the checksum of an iso image written to CD/DVD instead
of calculating it afterwards.

BUG: 82843
BUG: 134021


 M  +1 -0      jobs/k3bclonetocreader.cpp  
 M  +27 -4     jobs/k3biso9660imagewritingjob.cpp  
 M  +3 -0      jobs/k3biso9660imagewritingjob.h  
 M  +39 -39    jobs/k3bisoimageverificationjob.cpp  
 M  +2 -0      jobs/k3bisoimageverificationjob.h  
 M  +59 -12    tools/k3bchecksumpipe.cpp  
 M  +22 -3     tools/k3bchecksumpipe.h  
 M  +8 -2      tools/k3bmd5job.cpp  


--- trunk/extragear/multimedia/k3b/libk3b/jobs/k3bclonetocreader.cpp #597650:597651
@@ -78,6 +78,7 @@
 
     struct tocheader* th = (struct tocheader*)buffer;
     int dataLen = K3bDevice::from2Byte( th->len ) + 2;  // the len field does not include it's own length
+    kdDebug() << "(K3bCloneTocReader) dataLen+2 = " << dataLen << endl;
 
     if( th->first != 1 ) {
       kdDebug() << "(K3bCloneTocReader) first session != 1" << endl;
--- trunk/extragear/multimedia/k3b/libk3b/jobs/k3biso9660imagewritingjob.cpp #597650:597651
@@ -27,6 +27,7 @@
 #include <k3bcore.h>
 #include <k3bversion.h>
 #include <k3bexternalbinmanager.h>
+#include <k3bchecksumpipe.h>
 
 #include <kdebug.h>
 #include <kconfig.h>
@@ -40,6 +41,14 @@
 #include <qapplication.h>
 
 
+class K3bIso9660ImageWritingJob::Private
+{
+public:
+  K3bChecksumPipe checksumPipe;
+  QFile imageFile;
+};
+
+
 K3bIso9660ImageWritingJob::K3bIso9660ImageWritingJob( K3bJobHandler* hdl )
   : K3bBurnJob( hdl ),
     m_writingMode(K3b::WRITING_MODE_AUTO),
@@ -53,11 +62,13 @@
     m_copies(1),
     m_verifyJob(0)
 {
+  d = new Private;
 }
 
 K3bIso9660ImageWritingJob::~K3bIso9660ImageWritingJob()
 {
   delete m_tocFile;
+  delete d;
 }
 
 
@@ -95,6 +106,8 @@
     return;
   }
 
+  d->checksumPipe.close();
+
   if( success ) {
     if( !m_simulate && m_verifyData ) {
       emit burning(false);
@@ -112,6 +125,7 @@
       }
       m_verifyJob->setDevice( m_device );
       m_verifyJob->setImageFileName( m_imagePath );
+      m_verifyJob->setImageMD5Sum( d->checksumPipe.checksum() );
       if( m_copies == 1 )
 	emit newTask( i18n("Verifying written data") );
       else
@@ -253,9 +267,18 @@
     return;
   }
 
+  // we simply always calculate the checksum, thus making the code simpler
+  d->imageFile.close();
+  d->imageFile.setName( m_imagePath );
+  d->imageFile.open( IO_ReadOnly );
+  d->checksumPipe.close();
+  d->checksumPipe.readFromFd( d->imageFile.handle() );
+
   if( prepareWriter( media ) ) {
     emit burning(true);
     m_writer->start();
+    d->checksumPipe.writeToFd( m_writer->fd(), true );
+    d->checksumPipe.open( K3bChecksumPipe::MD5, true );
   }
   else {
     m_finished = true;
@@ -319,7 +342,8 @@
       else
 	writer->addArgument("-data");
 
-      writer->addArgument( m_imagePath );
+      // read from stdin
+      writer->addArgument( "-" );
 
       m_writer = writer;
     }
@@ -349,7 +373,7 @@
 	  *s << "\n";
 	  *s << "TRACK MODE1" << "\n";
 	}
-	*s << "DATAFILE \"" << m_imagePath << "\" 0 \n";
+	*s << "DATAFILE \"-\" 0 \n";
 
 	m_tocFile->close();
       }
@@ -382,13 +406,12 @@
     writer->setSimulate( m_simulate );
     writer->setBurnSpeed( m_speed );
     writer->setWritingMode( m_writingMode == K3b::DAO ? K3b::DAO : 0 );
-    writer->setImageToWrite( m_imagePath );
+    writer->setImageToWrite( QString::null ); // read from stdin
     writer->setCloseDvd( !m_noFix );
 
     m_writer = writer;
   }
 
-
   connect( m_writer, SIGNAL(infoMessage(const QString&, int)), this, SIGNAL(infoMessage(const QString&, int)) );
   connect( m_writer, SIGNAL(nextTrack(int, int)), this, SLOT(slotNextTrack(int, int)) );
   connect( m_writer, SIGNAL(percent(int)), this, SLOT(slotWriterPercent(int)) );
--- trunk/extragear/multimedia/k3b/libk3b/jobs/k3biso9660imagewritingjob.h #597650:597651
@@ -90,6 +90,9 @@
   int m_currentCopy;
 
   K3bIsoImageVerificationJob* m_verifyJob;
+
+  class Private;
+  Private* d;
 };
 
 #endif
--- trunk/extragear/multimedia/k3b/libk3b/jobs/k3bisoimageverificationjob.cpp #597650:597651
@@ -1,10 +1,10 @@
 /* 
  *
  * $Id$
- * Copyright (C) 2003 Sebastian Trueg <trueg@k3b.org>
+ * Copyright (C) 2003-2006 Sebastian Trueg <trueg@k3b.org>
  *
  * This file is part of the K3b project.
- * Copyright (C) 1998-2004 Sebastian Trueg <trueg@k3b.org>
+ * Copyright (C) 1998-2006 Sebastian Trueg <trueg@k3b.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -40,6 +40,7 @@
 
   bool canceled;
   bool needToCalcMd5;
+  bool imageMd5SumDone;
   K3bMd5Job* md5Job;
   K3bDevice::Device* device;
   QString imageFileName;
@@ -101,12 +102,19 @@
 
   d->canceled = false;
   d->needToCalcMd5 = d->imageMd5Sum.isEmpty();
+  d->imageMd5SumDone = false;
 
+  d->imageSize = K3b::filesize( d->imageFileName );
+  if( d->imageSize == (KIO::filesize_t)0 ) {
+    emit infoMessage( i18n("Unable to determine size of file %1.").arg(d->imageFileName), ERROR );
+    finishVerification(false);
+  }
+
   // first we need to reload and mount the device
   emit newTask( i18n("Reloading the medium") );
 
   connect( K3bDevice::reload( d->device ), SIGNAL(finished(bool)),
-	     this, SLOT(slotMediaReloaded(bool)) );
+	   this, SLOT(slotMediaReloaded(bool)) );
 }
 
 
@@ -117,27 +125,42 @@
 			 i18n("Unable to Close the Tray") );
 
   if( d->needToCalcMd5 ) {
-    emit newTask( i18n("Reading original data") );
-    
-    // start it
-    d->md5Job->setFile( d->imageFileName );
-    d->md5Job->start();
+    calculateImageChecksum();
   }
   else {
-    slotMd5JobFinished( true );
+    d->imageMd5SumDone = true;
+    calculateWrittenDataChecksum();
   }
 }
 
 
-void K3bIsoImageVerificationJob::slotMd5JobFinished( bool success )
+void K3bIsoImageVerificationJob::calculateImageChecksum()
 {
-  if( d->canceled ) {
+  emit newTask( i18n("Reading original data") );
+  d->md5Job->setFile( d->imageFileName );
+  d->md5Job->start();
+}
+
+
+void K3bIsoImageVerificationJob::calculateWrittenDataChecksum()
+{
+  emit newTask( i18n("Reading written data") );
+  if( !d->device->open() ) {
+    emit infoMessage( i18n("Unable to open device %1.").arg(d->device->blockDeviceName()), ERROR );
     finishVerification(false);
   }
+  else {
+    d->md5Job->setDevice( d->device );
+    d->md5Job->setMaxReadSize( d->imageSize );
+    d->md5Job->start();
+  }
+}
 
-  if( success ) {
 
-    if( !d->imageMd5Sum.isEmpty() ) {
+void K3bIsoImageVerificationJob::slotMd5JobFinished( bool success )
+{
+  if( success && !d->canceled ) {
+    if( d->imageMd5SumDone ) {
       // compare the two sums
       if( d->imageMd5Sum != d->md5Job->hexDigest() ) {
 	emit infoMessage( i18n("Written data differs from original."), ERROR );
@@ -149,32 +172,9 @@
       }
     }
     else {
-      
       d->imageMd5Sum = d->md5Job->hexDigest();
-
-      //
-      // now we need to calculate the md5sum of the written image
-      // since it is possible that the image has been padded while writing we need
-      // the image filesize to make sure we do not compare too much data and thus get
-      // a wrong result
-      //
-
-      d->imageSize = K3b::filesize( d->imageFileName );
-      if( d->imageSize == (KIO::filesize_t)0 ) {
-	emit infoMessage( i18n("Unable to determine size of file %1.").arg(d->imageFileName), ERROR );
-	finishVerification(false);
-      }
-      else if( !d->device->open() ) {
-	emit infoMessage( i18n("Unable to open device %1.").arg(d->device->blockDeviceName()), ERROR );
-	finishVerification(false);
-      }
-      else {
-	// start the written data check
-	emit newTask( i18n("Reading written data") );
-	d->md5Job->setDevice( d->device );
-	d->md5Job->setMaxReadSize( d->imageSize );
-	d->md5Job->start();
-      }
+      d->imageMd5SumDone = true;
+      calculateWrittenDataChecksum();
     }
   }
   else {
@@ -186,7 +186,7 @@
 
 void K3bIsoImageVerificationJob::slotMd5JobProgress( int p )
 {
-  if( d->needToCalcMd5 && !d->imageMd5Sum.isEmpty() )
+  if( d->needToCalcMd5 && d->imageMd5SumDone )
     emit percent( 50 + p/2 );
   else if( d->needToCalcMd5 )
     emit percent( p/2 );
--- trunk/extragear/multimedia/k3b/libk3b/jobs/k3bisoimageverificationjob.h #597650:597651
@@ -39,6 +39,8 @@
   void setImageMD5Sum( const QCString& );
 
  private slots:
+  void calculateImageChecksum();
+  void calculateWrittenDataChecksum();
   void slotMediaReloaded( bool success );
   void slotMd5JobFinished( bool success );
   void slotMd5JobProgress( int p );
--- trunk/extragear/multimedia/k3b/libk3b/tools/k3bchecksumpipe.cpp #597650:597651
@@ -29,7 +29,10 @@
 {
 public:
   Private() :
-    fdToWriteTo(-1) {
+    fdToReadFrom(-1),
+    fdToWriteTo(-1),
+    closeFdToReadFrom(false),
+    closeFdToWriteTo(false) {
   }
 
   void run() {
@@ -37,19 +40,20 @@
     buffer.resize( 10*2048 );
     ssize_t r = 0;
     ssize_t total = 0;
-    while( ( r = ::read( pipeIn.out(), buffer.data(), buffer.size() ) ) > 0 ) {
+    while( ( r = ::read( readFd(), buffer.data(), buffer.size() ) ) > 0 ) {
 
       // write it out
       ssize_t w = 0;
       ssize_t ww = 0;
       while( w < r ) {
-	if( ( ww = ::write( fdToWriteTo == -1 ? pipeOut.in() : fdToWriteTo,
-			    buffer.data()+w, r-w ) ) > 0 ) {
+	if( ( ww = ::write( writeFd(), buffer.data()+w, r-w ) ) > 0 ) {
 	  w += ww;
 	}
 	else {
 	  kdDebug() << "(K3bChecksumPipe) write failed." << endl;
-	  break;
+	  if( closeWhenDone )
+	    close();
+	  return;
 	}
       }
 
@@ -59,6 +63,9 @@
       total += r;
     }
     kdDebug() << "(K3bChecksumPipe) thread done: " << r << " (total bytes: " << total << ")" << endl;
+
+    if( closeWhenDone )
+      close();
   }
 
   void update( const char* in, int len ) {
@@ -77,12 +84,44 @@
     }
   }
 
+  int readFd() const {
+    if( fdToReadFrom == -1 )
+      return pipeIn.out();
+    else
+      return fdToReadFrom;
+  }
+
+  int writeFd() const {
+    if( fdToWriteTo == -1 )
+      return pipeOut.in();
+    else
+      return fdToWriteTo;
+  }
+
+  void close() {
+    pipeIn.close();
+    pipeOut.close();
+
+    if( fdToWriteTo != -1 &&
+	closeFdToWriteTo )
+      ::close( fdToWriteTo );
+
+    if( fdToReadFrom != -1 &&
+	closeFdToReadFrom )
+      ::close( fdToReadFrom );
+  }
+
   int checksumType;
 
+  int fdToReadFrom;
   int fdToWriteTo;
   K3bPipe pipeIn;
   K3bPipe pipeOut;
 
+  bool closeWhenDone;
+  bool closeFdToReadFrom;
+  bool closeFdToWriteTo;
+
   KMD5 md5;
 
   QByteArray buffer;
@@ -101,13 +140,15 @@
 }
 
 
-bool K3bChecksumPipe::open( int type )
+bool K3bChecksumPipe::open( int type, bool closeWhenDone )
 {
-  close();
+  if( d->running() )
+    return false;
 
   d->checksumType = type;
+  d->closeWhenDone = closeWhenDone;
 
-  if( !d->pipeIn.open() ) {
+  if( d->fdToReadFrom == -1 && !d->pipeIn.open() ) {
     return false;
   }
 
@@ -126,17 +167,22 @@
 void K3bChecksumPipe::close()
 {
   d->pipeIn.closeIn();
-
   d->wait();
+  d->close();
+}
 
-  d->pipeIn.close();
-  d->pipeOut.close();
+
+void K3bChecksumPipe::readFromFd( int fd, bool close )
+{
+  d->fdToReadFrom = fd;
+  d->closeFdToReadFrom = close;
 }
 
 
-void K3bChecksumPipe::writeToFd( int fd )
+void K3bChecksumPipe::writeToFd( int fd, bool close )
 {
   d->fdToWriteTo = fd;
+  d->closeFdToWriteTo = close;
 }
 
 
@@ -161,3 +207,4 @@
 
   return QCString();
 }
+
--- trunk/extragear/multimedia/k3b/libk3b/tools/k3bchecksumpipe.h #597650:597651
@@ -16,7 +16,6 @@
 #ifndef _K3B_CHECKSUM_PIPE_H_
 #define _K3B_CHECKSUM_PIPE_H_
 
-
 #include <qcstring.h>
 
 /**
@@ -36,18 +35,38 @@
   /**
    * Opens the pipe and thus starts the 
    * checksum calculation
+   *
+   * \param closeWhenDone If true the pipes will be closed
+   *        once all data has been read.
    */
-  bool open( int type = MD5 );
+  bool open( int type = MD5, bool closeWhenDone = false );
 
   /**
    * Close the pipe and finish the checksum.
    */
   void close();
 
-  void writeToFd( int fd );
+  /**
+   * Set the file descriptor to read from. If this is -1 (the default) then
+   * data has to be piped into the in() file descriptor.
+   *
+   * \param fd The file descriptor to read from.
+   * \param close If true the reading file descriptor will be closed on a call to close()
+   */
+  void readFromFd( int fd, bool close = false );
 
   /**
+   * Set the file descriptor to write to. If this is -1 (the default) then
+   * data has to read from the out() file descriptor.
+   *
+   * \param fd The file descriptor to write to.
+   * \param close If true the reading file descriptor will be closed on a call to close()
+   */
+  void writeToFd( int fd, bool close = false );
+
+  /**
    * The file descriptor to write into
+   * Only valid if no fd to read from has been set
    */
   int in() const;
 
--- trunk/extragear/multimedia/k3b/libk3b/tools/k3bmd5job.cpp #597650:597651
@@ -1,10 +1,10 @@
 /* 
  *
  * $Id$
- * Copyright (C) 2003 Sebastian Trueg <trueg@k3b.org>
+ * Copyright (C) 2003-2006 Sebastian Trueg <trueg@k3b.org>
  *
  * This file is part of the K3b project.
- * Copyright (C) 1998-2004 Sebastian Trueg <trueg@k3b.org>
+ * Copyright (C) 1998-2006 Sebastian Trueg <trueg@k3b.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -114,6 +114,12 @@
   else
     d->imageSize = 0;
 
+  if( d->device ) {
+    //
+    // Let the drive determine the optimal reading speed
+    //
+    d->device->setSpeed( 0xffff, 0xffff );
+  }
     
   d->md5.reset();
   d->finished = false;

nice work sebas. thanx.