----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104213/ -----------------------------------------------------------
(Updated March 16, 2012, 4:01 p.m.) Review request for Amarok and Teo Mrnjavac. Changes ------- gui string changes as suggested by Bart & Teo. I'll probably merge this later today. Description (updated) ------- Rework transcoding: remember encoder, transcode on move, cleaner code This is a major rework of transcoding feature that brings following user-visible changes to Amarok: * Amarok can remember preferred transcoding configuration per each collection that supports transcoding. Therefore, the "Use default configuration" work-around can go away and the "Transcode or copy?" dialog can (and is) be one-step now. This preference can be changed in configuration. * Transcoding is now supported even during the move operation. No worries, only successfully transcoded tracks are removed from their original location. * Only formats playable on the target collection are offered. Already used & tested in yet-to-be-merged iPod collection rewrite. * The "Organize Tracks" dialog title and progress bar operation name now more verbosely describe actual operation to prevent user mistakes. * Double-transcode when ripping audio CDs that caused failures is avoided. (ChangeLog entry for this was miscredited to my earilier commit) Technically, following changes are made: * many methods that accepted optional TranscodingConfiguration now either have it mandatory or not at all. * TranscodingConfiguration's NULL_CODEC was splitted to JUST_COPY and INVALID along with convenience methods isValid() and isJustCopy(). This simplifies logic in many methods. * CollectionLocation::prepare{Copy,Move}() now don't have optional TranscodingConfiguration parameter. Depending on target collection, CollectionLocation determines it automatically or asks user in showSourceDialog() (overridable). AudioCdCollectionLocation already overrides it. * Collections that support transcoding now should expose TranscodeCapability which is used to a) indicate that transcoding is supported; b) query which file formats are playable on target collection; c) read & save & unset preferred transcoding parameters. Why the hell the new Capability? ================================ Many Amarok devs dislike the concept of capabilities[1]. Why the hell I introduced the new one? In ideal world Amarok would be able to transcode everything regardless of the target collection. This is however not doable witch current copyUrlToCollection() design - target collection needs to do non-trivial things such as re-reading file tags, accounting for different file name and space requirements etc. See my comments in [1]. We therefore need a way for target collection to indicate it supports transcoding (in order not to fool user). Some collection locations such as TrashCollectionLocation should even intentionally disallow transcoding. Additionally, we want to be able to query supported destination file formats, to save preferred transcoding paremeters etc. I simply didn't want to pollute already over-crowded CollectionLocation with three more methods used by only a few subclasses. On the other hand, TranscodeCapability is not the central idea of this patch and I can factor it into CollectionLocation should there be a voice supporting it. v2 patch version: gui string changes as suggested by Bart & Teo [1] https://git.reviewboard.kde.org/r/103752/ FEATURE: 280526 FEATURE: 264681 CCBUG: 291722 BUG: 263775 FIXED-IN: 2.6 REVIEW: 104213 DIGEST: Feature: much improved transcoding Transcoding::Property: remove NUMERIC, LIST, TEXT types These types were not used since Teo reworked all encoders to use the TRADEOFF type. Remove them and associated code to make codebase cleaner so that new code doesn't need to introduce case statements in switches that will be never used, thus error-prone. Individual types can be resurrected from this commit if there is a need for them in future. CollectionLocation: display source dialog in next mainloop iteration This is to make CollectionLocation::prepareCopy/Move() return fast as it advertises and not after several seconds when a modal dialog is shown. Diffs (updated) ----- ChangeLog bd777eadf39aec71efb74dfed7502f564553d998 src/CMakeLists.txt 4241e69000c8b7fb944e4c86ddff3128829fb381 src/browsers/CollectionTreeView.h 26ac68a55242d52cdb1d789cef839643b56ccf5a src/browsers/CollectionTreeView.cpp 7b3dd81b1cfd0fbb4d74b7eb71552a4ad92d74e5 src/browsers/filebrowser/FileView.h 890b9f458e7ae504a52019b8f41e3e6d2ba218a5 src/browsers/filebrowser/FileView.cpp 425641af15e66c2670bce719eff1615f416ad6b7 src/configdialog/dialogs/CollectionConfig.cpp 7704fb9fab5a09c4635c1ec7526ae05047df0c6f src/configdialog/dialogs/CollectionConfig.ui d0ccdb33e8e54ecf4db205e10dbd8396eac488ad src/core-impl/collections/db/sql/SqlCollection.h 3a96bea1aa21761f12d956f7905f87808e0efc4a src/core-impl/collections/db/sql/SqlCollection.cpp 79714153661e50b70885b3e82cb6529b79ff98e2 src/core-impl/collections/db/sql/SqlCollectionLocation.h 1db72726d041713158be0fe91a93be3b1044b70e src/core-impl/collections/db/sql/SqlCollectionLocation.cpp 2c33da62565a9be4b5710cce590bac99d28b47ae src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.h 24bb306c5b28a6d21f66f598f4caccbbb60f5bf8 src/core-impl/collections/support/CollectionLocationDelegateImpl.h 08a1762bb4c72b7ba32658651e00bf356e2cffba src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp 23986570a42ad78557a394581233921abc668a8f src/core-impl/collections/support/PlaylistCollectionLocation.h 967d2150cf2c76f563dac0ff5396e84448826aed src/core-impl/collections/support/TrashCollectionLocation.h a9c92495abebd13506ed52a185355f21b55ee347 src/core-impl/collections/umscollection/UmsCollectionLocation.h 3fc0af405a8d47374afacdf5c1190d112b126d21 src/core/CMakeLists.txt 4db2105f08246898585bbe38ba4fbe0d006bd138 src/core/capabilities/Capability.h 42ffd7e89fd67d1b1b1b0b6f3a8ddc35cde5943e src/core/capabilities/TranscodeCapability.h PRE-CREATION src/core/capabilities/TranscodeCapability.cpp PRE-CREATION src/core/collections/CollectionLocation.h 35480b2000cccf9ece99b7654ff3852087b3144e src/core/collections/CollectionLocation.cpp 9b258ce9a19ab22f7027b674da4b76d1f15d973b src/core/collections/CollectionLocationDelegate.h c49a7445e260665e508795ae8dbee4ac9f271f71 src/core/transcoding/TranscodingConfiguration.h 378fe12ffa86764fe3ecf0cc5f2a38d8a47a0561 src/core/transcoding/TranscodingConfiguration.cpp 40f09be71ee0c75cbeeb6d3a4702c2a6f5ec3b99 src/core/transcoding/TranscodingController.h e9b04df6478e1f18d1e8c7d6d9859e6c1c86d9f3 src/core/transcoding/TranscodingController.cpp e16b6fe7d3d60621ef0a237951ac038f7846b51e src/core/transcoding/TranscodingDefines.h 4ba3e2ae9de710aa7a5f446eddf565e9f5138e4c src/core/transcoding/TranscodingFormat.h 5bf4e6098be4f3e08dcd45c48fdd264418660293 src/core/transcoding/TranscodingProperty.h 928854baa31eb4e78b3fe80768eb4f9811a0f5bf src/core/transcoding/TranscodingProperty.cpp 2b6752cfbc39e37880b05930dd9e797d60200c97 src/core/transcoding/formats/TranscodingNullFormat.h 96393e80f9a74a34a6858fcc114f2719089fac71 src/core/transcoding/formats/TranscodingNullFormat.cpp 98f09d1853833d2fd8b8de0603612ae337c6ef52 src/services/magnatune/MagnatuneCollectionLocation.cpp d75ea9edd0af9317b66f223961d325635fb26e33 src/services/mp3tunes/Mp3tunesServiceCollectionLocation.h c168a1f7bb07e798702b01f488912fd0b2c191c5 src/transcoding/CMakeLists.txt 06d5be85075581e979dc5fd8b411286200049846 src/transcoding/TranscodingAssistantDialog.h 718c01d40fed0113087b90e425a9102b365076cb src/transcoding/TranscodingAssistantDialog.cpp b64e74b9ee75c8b597a07c008a4e161333bb0d86 src/transcoding/TranscodingAssistantDialog.ui 912a1c2d636f4b24b1efd94185c0ae41f11ee96a src/transcoding/TranscodingOptionsStackedWidget.cpp 54c936dd041d9e23820afc8dbde662b5bb0dcd46 src/transcoding/TranscodingPropertyComboBoxWidget.h ecab1583fcb27ab78497e9c56f8c26a0e9a8b05a src/transcoding/TranscodingPropertyComboBoxWidget.cpp dbb2462f526ad5b47c14efa5c0f0983db296cb20 src/transcoding/TranscodingPropertySliderWidget.cpp 0a49da06127d9fa4dde9f8df95650e1a9a5ed6bc src/transcoding/TranscodingPropertySpinBoxWidget.h f1fa7f561c518f7420fc904252e35e8c107dd707 src/transcoding/TranscodingPropertySpinBoxWidget.cpp dcc34efa2c66e1f7be2af64524be238cd4f2fe8e src/transcoding/TranscodingPropertyWidget.cpp 4767c5208542b3815a4ca57023150bc2f5c7cf42 src/transcoding/TranscodingSelectConfigWidget.h PRE-CREATION src/transcoding/TranscodingSelectConfigWidget.cpp PRE-CREATION tests/core-impl/collections/db/sql/TestSqlCollectionLocation.cpp 7e90d1cd5acc6ef101897ad277c3a2f992bf3150 tests/core/collections/MockCollectionLocationDelegate.h a58ca4b344e98f6b75da19cdd6b38172ecc95f59 Diff: http://git.reviewboard.kde.org/r/104213/diff/ Testing ------- Works, both with Local Collection and in-the-works iPod collection. Screenshots (updated) ----------- Better Organize dialog title http://git.reviewboard.kde.org/r/104213/s/456/ Changes to the Configure Collection dialog http://git.reviewboard.kde.org/r/104213/s/467/ Revamped Transcode dialog http://git.reviewboard.kde.org/r/104213/s/468/ Thanks, Matěj Laitl
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel