> On Jan. 21, 2012, 11:56 a.m., Ralf Engels wrote: > > No obvious technical issues. > > > > However, I am wondering: Is there really a transcoding capability? > > I mean, we should be able to transcode everything independent of the > > collection it's in. > > > > Also, I don't like the concept of the capabilities. The only reason to have > > these capabilities is to keep the API binary compatible. > > But in Amarok we don't really care about that for now.
> However, I am wondering: Is there really a transcoding capability? > I mean, we should be able to transcode everything independent of the > collection it's in. Yes, I originally thought this would be the right & best approach. Thanks to CollectionLocation::copyUrlsToCollection( const QMap<Meta::TrackPtr, KUrl> ... ) we could transcode tracks somewhere in CollectionLocation and just replace the KUrls with the transcoded ones. But it would have some drawbacks: * motivation for this change was transcoding for iPod collection rewrite: transferring tracks onto iPod is usually slow (nearly as slow as transcoding). With such a 2-step process it would take twice the time. (because currently it can be done in parallel) * you would be essentially copying the files twice (witout greater CollectionLocation redesign) * many collection locations just copy source track metadata such as bitrate, length, filetype etc. This would need to be dealed with. * I'd still want to have ability to save per-destinaiton-collection preferred transcoding configuration. Collection locations would then have to provide something like {get,has}SavedConfiguration and saveConfiguration. (we don't want this to be handed universally, for example UmsCollection and iPod collection want to store this preferrence on the device) * The same applies to playableFileTypes() (altough this would maybe fit into CollectionLocation directly) Therefore I came to the conslusion that each CollectionLocation should implement transcoding in its copyUrlsToCollection() method. This is not really hard, Transcoding::Job facilitates it. So the reason to put this into a capability was to have it self-contained and do not add additional methods to over-crowded CollectionLocation of Collection. But I will happily implement a better design should some suggest such. - Matěj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103752/#review9975 ----------------------------------------------------------- On Jan. 21, 2012, 12:47 a.m., Matěj Laitl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103752/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2012, 12:47 a.m.) > > > Review request for Amarok and Teo Mrnjavac. > > > Description > ------- > > Rework transcoding: CollectionLocation asks user, not caller of prepareCopy() > > This moves the "get transcoding configuration" from callers of > prapareCopy() to CollectionLocation itself, resulting in hopefully more > predictable user experience. > > New capability, TranscodeCapability is created and supposed to be > provided by collections to indicate source CollectionLocation it should > display the transcoding assistant dialog. Default implementation of it > has no pure virtual methods and SqlCollection is changed to provide it, > making this change invisible to user for now. > > TranscodeCapability can also tell what formats will be playable on > target collection (so that meaningless codecs can be disabled in the > dialog) and mainly it allows target collections to store their > preferred transcoding configuration so that the user is not bothered > with the dialog every time. (the NULL_CODEC option can store a > preference "just copy") No collection is currently able to do this, > but I will implement it for SqlCollection and rewrite of IpodCollection. > > IpodCollection rewrite (not yet in master) is the other coll. that > provides the capability, it already implements playableFileTypes() > > TranscodingAssistantDialog is also tweaked so that it disables (not > hides) unavailable transcoding options. (with info in a tooltip) Some > core transcoding classes are cleaned up. > > Following bugs are still valid, but this is a first step to solving > them: > CCBUG: 280526 > CCBUG: 264681 > CCBUG: 291722 > DIGEST: groundwork for more convenient transcoding in Amarok > > > Diffs > ----- > > src/CMakeLists.txt aed51026af8b4b7e826ba0f38c1bce328f78089a > src/browsers/CollectionTreeView.h b70f99d811d1f7f271ec79298c3b0140fbd0ede4 > src/browsers/CollectionTreeView.cpp > c9069c770fd28fe16e76b1af132f3c7dff4c82d3 > src/browsers/filebrowser/FileView.h > 890b9f458e7ae504a52019b8f41e3e6d2ba218a5 > src/browsers/filebrowser/FileView.cpp > b23abc22a91bb8ca7bc8339cc873153c7cb179eb > src/core-impl/collections/db/sql/SqlCollection.cpp > ace308b3831deaf51c91dd892e224060d9c03461 > src/core-impl/collections/support/CollectionLocationDelegateImpl.h > 12b975fc7d5c5d56e314f3fce4384eb559e88274 > src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp > fb7c18f6fef5ef634ef1a40cea604f12a379cfc6 > src/core/CMakeLists.txt 6a876e842fc2551763848ebfd3f09a7d35fcc7a6 > src/core/capabilities/Capability.h b38d9166797658c99a0162e6dfa7944d47b98de5 > src/core/capabilities/TranscodeCapability.h PRE-CREATION > src/core/capabilities/TranscodeCapability.cpp PRE-CREATION > src/core/collections/CollectionLocation.h > 375a858aae47b27b5cb9081567b3e384c05d97d8 > src/core/collections/CollectionLocation.cpp > 01d0f5b8100cd2ad7fb0d3a4fa6d6c4a61e89931 > src/core/collections/CollectionLocationDelegate.h > c49a7445e260665e508795ae8dbee4ac9f271f71 > 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/formats/TranscodingNullFormat.cpp > 98f09d1853833d2fd8b8de0603612ae337c6ef52 > src/transcoding/TranscodingAssistantDialog.h > 718c01d40fed0113087b90e425a9102b365076cb > src/transcoding/TranscodingAssistantDialog.cpp > b64e74b9ee75c8b597a07c008a4e161333bb0d86 > tests/core/collections/MockCollectionLocationDelegate.h > a58ca4b344e98f6b75da19cdd6b38172ecc95f59 > > Diff: http://git.reviewboard.kde.org/r/103752/diff/diff > > > Testing > ------- > > Works as before with SqlCollection, works with iPod collection rewrite that > also disables (gays-out) unplayable transcoding options. > > > Thanks, > > Matěj Laitl > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel