Review Request: Rework transcoding: CollectionLocation asks user, not caller of prepareCopy()

[Ralf Engels]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103752/#review9975
-----------------------------------------------------------


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.

- Ralf Engels


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120121/8981211b/attachment.html>