Hi Matěj*, *Side question: you refer to an "in-the-works" ipod collection. Is this work in the main amarok git repository? If not, can you tell me where to find it?
Thanks, Julian On Fri, Mar 9, 2012 at 3:31 PM, Matěj Laitl <ma...@laitl.cz> wrote: > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104213/ > Review request for Amarok and Teo Mrnjavac. > By Matěj Laitl. > Description > > 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. > > [1] https://git.reviewboard.kde.org/r/103752/ > > FEATURE: 280526 > FEATURE: 264681 > CCBUG: 291722 > BUG: 263775 > FIXED-IN: 2.6 > REVIEW: TODO > DIGEST: Feature: much improved transcoding > > -------------------------------------------------------------------------------------------------- > Next commit squelched for the purpose of review board > -------------------------------------------------------------------------------------------------- > > 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. > > -------------------------------------------------------------------------------------------------- > Next commit squelched for the purpose of review board > -------------------------------------------------------------------------------------------------- > > 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. > > Testing > > Works, both with Local Collection and in-the-works iPod collection. > > Diffs > > - ChangeLog (6f318678272275bb6de35fd8cd6355dd9c175f2d) > - 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 > (12b975fc7d5c5d56e314f3fce4384eb559e88274) > - src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp > (fb7c18f6fef5ef634ef1a40cea604f12a379cfc6) > - 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) > > View Diff <http://git.reviewboard.kde.org/r/104213/diff/> > Screenshots > [image: Better Organize dialog > title]<http://git.reviewboard.kde.org/r/104213/s/456/> [image: > Changes to the Configure Collection > dialog]<http://git.reviewboard.kde.org/r/104213/s/457/> [image: > Revamped Transcode dialog]<http://git.reviewboard.kde.org/r/104213/s/458/> > > _______________________________________________ > Amarok-devel mailing list > Amarok-devel@kde.org > https://mail.kde.org/mailman/listinfo/amarok-devel > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
