Review Request 109781: Bug #312407 - don't transcode from mp3 to mp3

[Anmol Ahuja]

> On April 21, 2013, 9:02 p.m., Matěj Laitl wrote:
> > src/transcoding/TranscodingSelectConfigWidget.h, line 62
> > <http://git.reviewboard.kde.org/r/109781/diff/8/?file=139736#file139736line62>
> >
> >     I don't think you set this value anywhere anymore. (there's a switch case for it, but it now newer fires AFAICS)
> 
> Anmol Ahuja wrote:
>     It is, when the save configuration isJustCopy() and when it's invalid.
>     
>     Either of those cases can be replaced by the three newly added enums, since the trackSelection value of INVALID( except when restoring the prev trackSelection, in which case DontChange is desirable ) and JUST_COPY configs is ignored, but I think that would look bad.
> 
> Matěj Laitl wrote:
>     Hmm, cannot these be expressed by JustCopy and Forget?

I think we can get rid of Forget and JustCopy. I don't understand why they're there, actually. Do they serve some purpose? An INVALID is pretty much equivalent to JUST_COPY.

Should I rewrite it like this:

if( isValid() )
{
    if( !isJustCopy() )
        ...
    else
        ...
}
else
    ...
It's more readable


- Anmol


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


On April 19, 2013, 1:47 p.m., Anmol Ahuja wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109781/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 1:47 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Added 3 checkbox in the TranscodingAssistantDialog:
> "Transcode all tracks" - transcode all tracks, the current default behavior
> "Ignore files which're already the selected format" - transcode only if source and destination file formats are different
> "Transcode only when needed for playability" - transcode only when needed for playability in the destination collection
> 
> 
> Diffs
> -----
> 
>   ChangeLog cc8b166 
>   src/core-impl/collections/db/sql/SqlCollectionLocation.cpp 8bc4552 
>   src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.h 3c2d9f2 
>   src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp 8a40c6c 
>   src/core-impl/collections/support/CollectionLocationDelegateImpl.h b59beec 
>   src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp f19cd9d 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.h 45ba596 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.cpp 2a30c9f 
>   src/core/collections/CollectionLocation.cpp 08f6910 
>   src/core/collections/CollectionLocationDelegate.h 6316d6f 
>   src/core/transcoding/TranscodingConfiguration.h 98b2bb8 
>   src/core/transcoding/TranscodingConfiguration.cpp f97a43d 
>   src/transcoding/TranscodingAssistantDialog.h 76287a7 
>   src/transcoding/TranscodingAssistantDialog.cpp 8348b12 
>   src/transcoding/TranscodingAssistantDialog.ui 2505bd3 
>   src/transcoding/TranscodingOptionsStackedWidget.h 9495d44 
>   src/transcoding/TranscodingOptionsStackedWidget.cpp cf4f328 
>   src/transcoding/TranscodingSelectConfigWidget.h aa5f196 
>   src/transcoding/TranscodingSelectConfigWidget.cpp adb593c 
>   tests/core/collections/MockCollectionLocationDelegate.h 5efbe2e7 
> 
> Diff: http://git.reviewboard.kde.org/r/109781/diff/
> 
> 
> Testing
> -------
> 
> Works as expected
> All build tests passed
> 
> 
> File Attachments
> ----------------
> 
> TranscodingAssistantDialog
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/04/18/TranscodingAssistantDialog_1.png
> 
> 
> Thanks,
> 
> Anmol Ahuja
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130421/7a2ab904/attachment.html>