Merge into trunk : duplicates-feature : Code : gallery-app

Status:	Work in progress
Proposed branch:	lp://qastaging/~franxesk/gallery-app/duplicates-feature
Merge into:	lp://qastaging/gallery-app
Diff against target:	682 lines (+291/-31) 15 files modified rc/qml/Components/DuplicatesDialog.qml (+107/-0) rc/qml/GalleryApplication.qml (+11/-1) rc/sql/9.sql (+3/-0) src/database/database.cpp (+5/-0) src/database/media-table.cpp (+74/-6) src/database/media-table.h (+8/-3) src/gallery-manager.cpp (+27/-3) src/gallery-manager.h (+6/-1) src/media-object-factory.cpp (+25/-6) src/media-object-factory.h (+3/-1) src/media/media-monitor.cpp (+4/-4) src/media/media-monitor.h (+2/-2) tests/unittests/mediaobjectfactory/tst_mediaobjectfactory.cpp (+2/-1) tests/unittests/stubs/gallery-manager_stub.cpp (+7/-1) tests/unittests/stubs/media-table_stub.cpp (+7/-2)
To merge this branch:	bzr merge lp://qastaging/~franxesk/gallery-app/duplicates-feature
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Günter Schwann (community)		2013-10-24	Needs Fixing on 2013-11-12
Review via email: mp+192570@code.qastaging.launchpad.net

Commit message

Start implementing duplicates' detection feature.

Description of the change

Start implementing duplicates' detection feature.

lp://qastaging/~franxesk/gallery-app/duplicates-feature updated on 2013-11-11

868. By Francisco Merino <email address hidden> on 2013-10-28

Merging latest changes from trunk.

869. By Francisco Merino on 2013-10-28

Included 'MediaTable::checkMissingMd5sum' method to calculate
the md5sums for the previously added images into the gallery.

870. By Francisco Merino on 2013-10-28

Modified 'MediaTable::createIdForMedia' and 'MediaTable::updateMedia'
methods in order to store the md5sum attribute of each image.

871. By Francisco Merino on 2013-10-28

Created new database revision file to include the md5sum attribute
into the MediaTable table.

872. By Francisco Merino on 2013-10-28

Included 'MediaTable::checkDuplicates' method in charge of the
duplicates search.

873. By Francisco Merino on 2013-10-28

Change 'checkDuplicates' method name to 'MediaTable::searchForDuplicates'.

874. By Francisco Merino on 2013-10-29

Minor corrections in the 'MediaTable::checkMissingMd5sum()' method.

875. By Francisco Merino on 2013-10-29

Updated the db with the md5sum after upgrading its schema.

Included the mechanism to detect duplicates before updating the gallery.

Updated the mechanism to add new media into the gallery in order to
include its md5sum.

Remove old mechanism due to performance execution.

876. By Francisco Merino on 2013-10-29

Included new methods' declaration in the header.

877. By Francisco Merino on 2013-10-29

Updated invocation of the new methods in order to correct some calling
errors.

878. By Francisco Merino on 2013-10-31

Updated UI in order to include the dialog notification for duplicates.

Other minor changes.

879. By Francisco Merino on 2013-10-31

Move the duplicates dialog to its own file.

880. By Francisco Merino on 2013-10-31

Facing some problems with the mechanism to pass params to a QML element
defined in its own file.

881. By Francisco Merino on 2013-11-04

Working on the UI.

882. By Francisco Merino on 2013-11-04

Merging latest changes from trunk.

883. By Francisco Merino on 2013-11-04

Updated the communication between GalleryApplication and DuplicatesDialog
in order to share information between each other.

884. By Francisco Merino on 2013-11-05

Trying to show the duplicate image inside the DuplicatesDialog.

885. By Francisco Merino on 2013-11-06

At the moment, the duplicates' detection mechanism is able to
detect one at a time, because the system doesn't allow the user
to include the duplicate into the database.

Included a new dialog to notify the user about the paths
of the duplicate and original pictures.

886. By Francisco Merino on 2013-11-07

I've finally been able to show the filenames using a ListModel.
I'm still unable to give a proper dimmensions to the dialog.

887. By Francisco Merino on 2013-11-07

Merging latest changes from trunk.

888. By Francisco Merino on 2013-11-08

Updated the DuplicatesDialog in order to show the key info just in one
dialog and ask the user about removing or keeping the duplicate media.

889. By Francisco Merino on 2013-11-08

Added the removeFile method to the stub.

To-Do:
- Set fixed dimensions to the dialog element in DuplicatesDialog.
- Create a method to search for duplicates on the db in order to
detect duplicates already stored in the db.

890. By Francisco Merino on 2013-11-11

Updated the 'DuplicatesDialog' dialog in order to set fixed a fixed width
for the labels showing the absolute filepaths.

Removed the 'DuplicatesPathsDialog' because it's not needed anymore.

Revision history for this message

Günter Schwann (schwann) wrote on 2013-11-11:

#

51 + Image {
Please set the source size for that image to the size of that item.
http://qt-project.org/doc/qt-5.1/qtquick/qml-qtquick2-image.html#sourceSize-prop
Big images won't be shown otherwise (exceeding the max. GL texture size), and for performance reasons.

138 + console.log("duplicates have been found");
139 + console.log("filename of the duplicate: " + newFilename);
140 + console.log("originals' filenames: " + oldFilenames);
Please remove that obsolete debugging code.

260 + // Are we really interested in the duplicates filenames?
261 + // or should we just notify the user that there are (unknown) duplicates of a given image?
I'd say that question is answered, so the comment can be removed

review: Needs Fixing

Reply

lp://qastaging/~franxesk/gallery-app/duplicates-feature updated on 2013-11-12

891. By Francisco Merino on 2013-11-12: - Removed old comments.
- Reallocated warning info about the duplicates' absolute filepaths.
- Set max values to the image's dimensions to be shown.

Revision history for this message

Günter Schwann (schwann) wrote on 2013-11-12:

#

When starting gallery on a Galaxy Nexus with about 270 photos in the ~/Pictures the UI is blocked for several seconds (like 10-20) before showing the first thumbnails. We need to show something like "Upgrading the database" to the user during that time.

But this not only happens the first time when upgrading the DB, but all the time at the startup. I guess there is a bug somewhere ...

When upgrading the DB, the new DB was not written properly for me (might be that's because I wasn't able to close gallery properly. You could sync the DB to disc directly after the upgrade.

void GalleryManager::onMediaItemAdded(QString file, bool lookForDuplicates) is running the the main (UI) thread. And as calculating the md5sum is expensive, it block's the UI thread for quite a while.
Can we for example calculate the md5sum in the MediaMonitor already, and pass it to that function in addition (optionally). So the md5sum is calculated in the separate thread.

In general please use at least 1000 photos on the desktop, to easily see performance issues (I would suggest to even use 10.000 - to have a comparable speed to the phone).

review: Needs Fixing

Reply

Unmerged revisions

891. By Francisco Merino on 2013-11-12

- Removed old comments.
- Reallocated warning info about the duplicates' absolute filepaths.
- Set max values to the image's dimensions to be shown.

890. By Francisco Merino on 2013-11-11

Updated the 'DuplicatesDialog' dialog in order to set fixed a fixed width
for the labels showing the absolute filepaths.

Removed the 'DuplicatesPathsDialog' because it's not needed anymore.

889. By Francisco Merino on 2013-11-08

Added the removeFile method to the stub.

To-Do:
- Set fixed dimensions to the dialog element in DuplicatesDialog.
- Create a method to search for duplicates on the db in order to
detect duplicates already stored in the db.

888. By Francisco Merino on 2013-11-08

Updated the DuplicatesDialog in order to show the key info just in one
dialog and ask the user about removing or keeping the duplicate media.

887. By Francisco Merino on 2013-11-07

Merging latest changes from trunk.

886. By Francisco Merino on 2013-11-07

I've finally been able to show the filenames using a ListModel.
I'm still unable to give a proper dimmensions to the dialog.

885. By Francisco Merino on 2013-11-06

At the moment, the duplicates' detection mechanism is able to
detect one at a time, because the system doesn't allow the user
to include the duplicate into the database.

Included a new dialog to notify the user about the paths
of the duplicate and original pictures.

884. By Francisco Merino on 2013-11-05

Trying to show the duplicate image inside the DuplicatesDialog.

883. By Francisco Merino on 2013-11-04

Updated the communication between GalleryApplication and DuplicatesDialog
in order to share information between each other.

882. By Francisco Merino on 2013-11-04

Merging latest changes from trunk.

gallery-app

Merge lp://qastaging/~franxesk/gallery-app/duplicates-feature into lp://qastaging/gallery-app

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers