Hi Matěj<b>,<br></b>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?<br><br>Thanks,<br>Julian<br><br><div class="gmail_quote">
On Fri, Mar 9, 2012 at 3:31 PM, Matěj Laitl <span dir="ltr"><<a href="mailto:matej@laitl.cz">matej@laitl.cz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div style="font-family:Verdana,Arial,Helvetica,Sans-Serif">
<table style="border:1px #c9c399 solid" bgcolor="#f9f3c9" cellpadding="8" width="100%">
<tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/104213/" target="_blank">http://git.reviewboard.kde.org/r/104213/</a>
</td>
</tr>
</tbody></table>
<br>
<table style="background-image:url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png');background-repeat:repeat-x;border:1px black solid" bgcolor="#fefadf" cellpadding="8" cellspacing="0" width="100%">
<tbody><tr>
<td>
<div>Review request for Amarok and Teo Mrnjavac.</div>
<div>By Matěj Laitl.</div>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" cellspacing="0" width="100%">
<tbody><tr>
<td>
<pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">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] <a href="https://git.reviewboard.kde.org/r/103752/" target="_blank">https://git.reviewboard.kde.org/r/103752/</a>
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.</pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" cellspacing="0" width="100%">
<tbody><tr>
<td>
<pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">Works, both with Local Collection and in-the-works iPod collection.</pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">
<li>ChangeLog <span style="color:grey">(6f318678272275bb6de35fd8cd6355dd9c175f2d)</span></li>
<li>src/CMakeLists.txt <span style="color:grey">(4241e69000c8b7fb944e4c86ddff3128829fb381)</span></li>
<li>src/browsers/CollectionTreeView.h <span style="color:grey">(26ac68a55242d52cdb1d789cef839643b56ccf5a)</span></li>
<li>src/browsers/CollectionTreeView.cpp <span style="color:grey">(7b3dd81b1cfd0fbb4d74b7eb71552a4ad92d74e5)</span></li>
<li>src/browsers/filebrowser/FileView.h <span style="color:grey">(890b9f458e7ae504a52019b8f41e3e6d2ba218a5)</span></li>
<li>src/browsers/filebrowser/FileView.cpp <span style="color:grey">(425641af15e66c2670bce719eff1615f416ad6b7)</span></li>
<li>src/configdialog/dialogs/CollectionConfig.cpp <span style="color:grey">(7704fb9fab5a09c4635c1ec7526ae05047df0c6f)</span></li>
<li>src/configdialog/dialogs/CollectionConfig.ui <span style="color:grey">(d0ccdb33e8e54ecf4db205e10dbd8396eac488ad)</span></li>
<li>src/core-impl/collections/db/sql/SqlCollection.h <span style="color:grey">(3a96bea1aa21761f12d956f7905f87808e0efc4a)</span></li>
<li>src/core-impl/collections/db/sql/SqlCollection.cpp <span style="color:grey">(79714153661e50b70885b3e82cb6529b79ff98e2)</span></li>
<li>src/core-impl/collections/db/sql/SqlCollectionLocation.h <span style="color:grey">(1db72726d041713158be0fe91a93be3b1044b70e)</span></li>
<li>src/core-impl/collections/db/sql/SqlCollectionLocation.cpp <span style="color:grey">(2c33da62565a9be4b5710cce590bac99d28b47ae)</span></li>
<li>src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.h <span style="color:grey">(24bb306c5b28a6d21f66f598f4caccbbb60f5bf8)</span></li>
<li>src/core-impl/collections/support/CollectionLocationDelegateImpl.h <span style="color:grey">(12b975fc7d5c5d56e314f3fce4384eb559e88274)</span></li>
<li>src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp <span style="color:grey">(fb7c18f6fef5ef634ef1a40cea604f12a379cfc6)</span></li>
<li>src/core-impl/collections/support/PlaylistCollectionLocation.h <span style="color:grey">(967d2150cf2c76f563dac0ff5396e84448826aed)</span></li>
<li>src/core-impl/collections/support/TrashCollectionLocation.h <span style="color:grey">(a9c92495abebd13506ed52a185355f21b55ee347)</span></li>
<li>src/core-impl/collections/umscollection/UmsCollectionLocation.h <span style="color:grey">(3fc0af405a8d47374afacdf5c1190d112b126d21)</span></li>
<li>src/core/CMakeLists.txt <span style="color:grey">(4db2105f08246898585bbe38ba4fbe0d006bd138)</span></li>
<li>src/core/capabilities/Capability.h <span style="color:grey">(42ffd7e89fd67d1b1b1b0b6f3a8ddc35cde5943e)</span></li>
<li>src/core/capabilities/TranscodeCapability.h <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/core/capabilities/TranscodeCapability.cpp <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/core/collections/CollectionLocation.h <span style="color:grey">(35480b2000cccf9ece99b7654ff3852087b3144e)</span></li>
<li>src/core/collections/CollectionLocation.cpp <span style="color:grey">(9b258ce9a19ab22f7027b674da4b76d1f15d973b)</span></li>
<li>src/core/collections/CollectionLocationDelegate.h <span style="color:grey">(c49a7445e260665e508795ae8dbee4ac9f271f71)</span></li>
<li>src/core/transcoding/TranscodingConfiguration.h <span style="color:grey">(378fe12ffa86764fe3ecf0cc5f2a38d8a47a0561)</span></li>
<li>src/core/transcoding/TranscodingConfiguration.cpp <span style="color:grey">(40f09be71ee0c75cbeeb6d3a4702c2a6f5ec3b99)</span></li>
<li>src/core/transcoding/TranscodingController.h <span style="color:grey">(e9b04df6478e1f18d1e8c7d6d9859e6c1c86d9f3)</span></li>
<li>src/core/transcoding/TranscodingController.cpp <span style="color:grey">(e16b6fe7d3d60621ef0a237951ac038f7846b51e)</span></li>
<li>src/core/transcoding/TranscodingDefines.h <span style="color:grey">(4ba3e2ae9de710aa7a5f446eddf565e9f5138e4c)</span></li>
<li>src/core/transcoding/TranscodingFormat.h <span style="color:grey">(5bf4e6098be4f3e08dcd45c48fdd264418660293)</span></li>
<li>src/core/transcoding/TranscodingProperty.h <span style="color:grey">(928854baa31eb4e78b3fe80768eb4f9811a0f5bf)</span></li>
<li>src/core/transcoding/TranscodingProperty.cpp <span style="color:grey">(2b6752cfbc39e37880b05930dd9e797d60200c97)</span></li>
<li>src/core/transcoding/formats/TranscodingNullFormat.h <span style="color:grey">(96393e80f9a74a34a6858fcc114f2719089fac71)</span></li>
<li>src/core/transcoding/formats/TranscodingNullFormat.cpp <span style="color:grey">(98f09d1853833d2fd8b8de0603612ae337c6ef52)</span></li>
<li>src/services/magnatune/MagnatuneCollectionLocation.cpp <span style="color:grey">(d75ea9edd0af9317b66f223961d325635fb26e33)</span></li>
<li>src/services/mp3tunes/Mp3tunesServiceCollectionLocation.h <span style="color:grey">(c168a1f7bb07e798702b01f488912fd0b2c191c5)</span></li>
<li>src/transcoding/CMakeLists.txt <span style="color:grey">(06d5be85075581e979dc5fd8b411286200049846)</span></li>
<li>src/transcoding/TranscodingAssistantDialog.h <span style="color:grey">(718c01d40fed0113087b90e425a9102b365076cb)</span></li>
<li>src/transcoding/TranscodingAssistantDialog.cpp <span style="color:grey">(b64e74b9ee75c8b597a07c008a4e161333bb0d86)</span></li>
<li>src/transcoding/TranscodingAssistantDialog.ui <span style="color:grey">(912a1c2d636f4b24b1efd94185c0ae41f11ee96a)</span></li>
<li>src/transcoding/TranscodingOptionsStackedWidget.cpp <span style="color:grey">(54c936dd041d9e23820afc8dbde662b5bb0dcd46)</span></li>
<li>src/transcoding/TranscodingPropertyComboBoxWidget.h <span style="color:grey">(ecab1583fcb27ab78497e9c56f8c26a0e9a8b05a)</span></li>
<li>src/transcoding/TranscodingPropertyComboBoxWidget.cpp <span style="color:grey">(dbb2462f526ad5b47c14efa5c0f0983db296cb20)</span></li>
<li>src/transcoding/TranscodingPropertySliderWidget.cpp <span style="color:grey">(0a49da06127d9fa4dde9f8df95650e1a9a5ed6bc)</span></li>
<li>src/transcoding/TranscodingPropertySpinBoxWidget.h <span style="color:grey">(f1fa7f561c518f7420fc904252e35e8c107dd707)</span></li>
<li>src/transcoding/TranscodingPropertySpinBoxWidget.cpp <span style="color:grey">(dcc34efa2c66e1f7be2af64524be238cd4f2fe8e)</span></li>
<li>src/transcoding/TranscodingPropertyWidget.cpp <span style="color:grey">(4767c5208542b3815a4ca57023150bc2f5c7cf42)</span></li>
<li>src/transcoding/TranscodingSelectConfigWidget.h <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/transcoding/TranscodingSelectConfigWidget.cpp <span style="color:grey">(PRE-CREATION)</span></li>
<li>tests/core-impl/collections/db/sql/TestSqlCollectionLocation.cpp <span style="color:grey">(7e90d1cd5acc6ef101897ad277c3a2f992bf3150)</span></li>
<li>tests/core/collections/MockCollectionLocationDelegate.h <span style="color:grey">(a58ca4b344e98f6b75da19cdd6b38172ecc95f59)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104213/diff/" style="margin-left:3em" target="_blank">View Diff</a></p>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/104213/s/456/" target="_blank"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/03/09/better_organize_dialog_title_400x100.png" style="border:1px black solid" alt="Better Organize dialog title"></a>
<a href="http://git.reviewboard.kde.org/r/104213/s/457/" target="_blank"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/03/09/changes_to_the_configure_dialog_400x100.png" style="border:1px black solid" alt="Changes to the Configure Collection dialog"></a>
<a href="http://git.reviewboard.kde.org/r/104213/s/458/" target="_blank"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/03/09/transcode_dialog_400x100.png" style="border:1px black solid" alt="Revamped Transcode dialog"></a>
</div>
</td>
</tr>
</tbody></table>
</div>
</div>
<br>_______________________________________________<br>
Amarok-devel mailing list<br>
<a href="mailto:Amarok-devel@kde.org">Amarok-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/amarok-devel" target="_blank">https://mail.kde.org/mailman/listinfo/amarok-devel</a><br>
<br></blockquote></div><br>