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 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
>
> W
