> On March 14, 2012, 11:10 a.m., Bart Cerneels wrote: > > Screenshot: Revamped Transcode dialog > > <http://git.reviewboard.kde.org> > > > > This is hard to understand and contains some language errors. Perhaps > > "Use this for next tracks" ? > > Matěj Laitl wrote: > I particularly suck at english. :) I wanted to justify that this option > will be used for both copying and moving and that the setting is used just > for the particular collection. Additionally, even the "Just copy" will be > remembered. All suggestions for the GUI string that will make these clear are > welcome. > > Bart Cerneels wrote: > It's still not clear to me. But perhaps just "Save settings" ?
How about "Remember this choice for next time" ? > On March 14, 2012, 11:10 a.m., Bart Cerneels wrote: > > Screenshot: Changes to the Configure Collection dialog > > <http://git.reviewboard.kde.org> > > > > If there are <= 3 options, don't use a combobox. > > Matěj Laitl wrote: > Yup, there are 2 or 3. Should I use radio buttons? > > Bart Cerneels wrote: > Use a groupbox: http://qt-project.org/doc/qt-4.8/widgets-groupbox.html > > It might make the dialog to tall. Might not be worth it for such an > advanced feature. Add a screenshot please so we can see what works. Please :) I think a combobox works fine here. Also I'd make the label "Transcode tracks" without a colon at the end, with the choices: "Always", "Ask before each transfer", "Never". - Teo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104213/#review11394 ----------------------------------------------------------- On March 9, 2012, 11:31 p.m., Matěj Laitl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104213/ > ----------------------------------------------------------- > > (Updated March 9, 2012, 11:31 p.m.) > > > Review request for Amarok and Teo Mrnjavac. > > > 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. > > > 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 > > Diff: http://git.reviewboard.kde.org/r/104213/diff/ > > > Testing > ------- > > Works, both with Local Collection and in-the-works iPod collection. > > > Screenshots > ----------- > > 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/457/ > Revamped Transcode dialog > http://git.reviewboard.kde.org/r/104213/s/458/ > > > Thanks, > > Matěj Laitl > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
