Bug 280526

Summary: Unable to select transcode when doing "move to collection"
Product: [Applications] amarok Reporter: lt.infiltrator
Component: TranscodingAssignee: Amarok Developers <amarok-bugs-dist>
Status: RESOLVED FIXED    
Severity: wishlist CC: aakashrajdahal, matej, teo
Priority: NOR    
Version: 2.5-git   
Target Milestone: 2.6   
Platform: Debian testing   
OS: Linux   
Latest Commit: Version Fixed In: 2.6
Sentry Crash Report:

Description lt.infiltrator 2011-08-21 14:46:15 UTC
Version:           2.4.3 (using KDE 4.6.5) 
OS:                Linux

In the file browser, if I select "copy to collection" I am given the option to transcode; but if I select "move to collection", I am not given the option.  As (so far) all the time I actually want to _move_ the file when transcoding it, and so have to manually delete files, this is both a nuisance and dangerous (I might delete files I have not actually transcoded yet, and thus lose them.)

Reproducible: Always

Steps to Reproduce:
Go to the file browser, and right click on a file.

Actual Results:  
See above.

Expected Results:  
Given me the same options when selecting "move to collection" as "copy to collection."

Actually, on second thoughts, there should only be one option, which is the same as "copy to collection" now, but in it (in that dialog box that shows up) there should be a tickbox that says "remove files once copy/transcoding is confirmed."
Comment 1 Aakash 2012-01-01 17:36:17 UTC
It seems more of a "wish" rather than a real codewise bug. Please tag accordingly.
Thanks
Comment 2 Myriam Schweingruber 2012-01-02 00:01:06 UTC
Not implemented yet, so this is indeed a wish.
Comment 3 Matěj Laitl 2012-01-21 00:05:21 UTC
I'm working on this.
Comment 4 Matěj Laitl 2012-03-16 20:10:03 UTC
Git commit 1a0287f7925d92a05a530ab83fffbc9afae67e86 by Matěj Laitl.
Committed on 21/01/2012 at 01:13.
Pushed by laitl into branch 'master'.

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/
Related: bug 264681, bug 263775, bug 291722
FIXED-IN: 2.6
REVIEW: 104213
DIGEST: Feature: much improved transcoding

M  +4    -1    ChangeLog
M  +1    -0    src/CMakeLists.txt
M  +10   -33   src/browsers/CollectionTreeView.cpp
M  +1    -2    src/browsers/CollectionTreeView.h
M  +1    -38   src/browsers/filebrowser/FileView.cpp
M  +0    -1    src/browsers/filebrowser/FileView.h
M  +21   -3    src/configdialog/dialogs/CollectionConfig.cpp
M  +33   -0    src/configdialog/dialogs/CollectionConfig.ui
M  +48   -12   src/core-impl/collections/db/sql/SqlCollection.cpp
M  +14   -1    src/core-impl/collections/db/sql/SqlCollection.h
M  +39   -18   src/core-impl/collections/db/sql/SqlCollectionLocation.cpp
M  +3    -1    src/core-impl/collections/db/sql/SqlCollectionLocation.h
M  +1    -1    src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.h
M  +17   -0    src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp
M  +3    -0    src/core-impl/collections/support/CollectionLocationDelegateImpl.h
M  +1    -1    src/core-impl/collections/support/PlaylistCollectionLocation.h
M  +2    -2    src/core-impl/collections/support/TrashCollectionLocation.h
M  +1    -1    src/core-impl/collections/umscollection/UmsCollectionLocation.h
M  +1    -0    src/core/CMakeLists.txt
M  +1    -0    src/core/capabilities/Capability.h
C  +5    -22   src/core/capabilities/TranscodeCapability.cpp [from: src/core/transcoding/TranscodingConfiguration.cpp - 073% similarity]
A  +84   -0    src/core/capabilities/TranscodeCapability.h     [License: GPL (v2+)]
M  +64   -21   src/core/collections/CollectionLocation.cpp
M  +19   -8    src/core/collections/CollectionLocation.h
M  +22   -0    src/core/collections/CollectionLocationDelegate.h
M  +112  -3    src/core/transcoding/TranscodingConfiguration.cpp
M  +41   -4    src/core/transcoding/TranscodingConfiguration.h
M  +17   -16   src/core/transcoding/TranscodingController.cpp
M  +21   -9    src/core/transcoding/TranscodingController.h
M  +11   -12   src/core/transcoding/TranscodingDefines.h
M  +3    -1    src/core/transcoding/TranscodingFormat.h
M  +4    -6    src/core/transcoding/formats/TranscodingNullFormat.cpp
M  +1    -1    src/core/transcoding/formats/TranscodingNullFormat.h
M  +2    -6    src/services/magnatune/MagnatuneCollectionLocation.cpp
M  +1    -1    src/services/mp3tunes/Mp3tunesServiceCollectionLocation.h
M  +1    -0    src/transcoding/CMakeLists.txt
M  +83   -75   src/transcoding/TranscodingAssistantDialog.cpp
M  +25   -9    src/transcoding/TranscodingAssistantDialog.h
M  +157  -296  src/transcoding/TranscodingAssistantDialog.ui
M  +3    -2    src/transcoding/TranscodingOptionsStackedWidget.cpp
A  +55   -0    src/transcoding/TranscodingSelectConfigWidget.cpp     [License: GPL (v2+)]
C  +42   -39   src/transcoding/TranscodingSelectConfigWidget.h [from: src/transcoding/TranscodingAssistantDialog.h - 050% similarity]
M  +1    -0    tests/core-impl/collections/db/sql/TestSqlCollectionLocation.cpp
M  +3    -0    tests/core/collections/MockCollectionLocationDelegate.h

http://commits.kde.org/amarok/1a0287f7925d92a05a530ab83fffbc9afae67e86