Bug 312407 - don't transcode from mp3 to mp3.
Summary: don't transcode from mp3 to mp3.
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Unclassified
Component: Transcoding (show other bugs)
Version: 2.6.0
Platform: Ubuntu Packages Linux
: HI wishlist (vote)
Target Milestone: 2.8
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-30 23:22 UTC by Thiago Jung Bauermann
Modified: 2013-04-27 21:49 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.8


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago Jung Bauermann 2012-12-30 23:22:12 UTC
I have MP3 and Ogg Vorbis music files. I intend to use the transcoding feature of Amarok to copy my collection to my device which understands only MP3. For files that are in the Ogg Vorbis encoding, Amarok clearly needs to reencode them. But it's also re-encoding the MP3 files, which is a waste of time and probably even degrades the audio quality of the music.

Please add an option to transcode only when it's necessary to change the file encoding.

Reproducible: Always

Steps to Reproduce:
1. Create USB Mass Storage device collection.
2. Configure it to transcode to MP3.
3. Copy an MP3 track to the collection.

Actual Results:  
The MP3 file that ends up in the device is not the same as the one on the local collection (the file size is very different, in some cases).

Expected Results:  
The MP3 file that ends up in the device should be exactly the same as the one on the local collection.
Comment 1 Matěj Laitl 2012-12-30 23:26:22 UTC
Yep, this is no my TODO list. Won't happen in 2.7 though. (string freeze)
Comment 2 Matěj Laitl 2012-12-30 23:26:41 UTC
*on my TODO list...
Comment 3 Thiago Jung Bauermann 2012-12-31 00:00:17 UTC
Great! Thanks for the nearly instantaneous reply.
Comment 4 Anmol Ahuja 2013-03-28 19:28:50 UTC
I'd like to work on this bug.
Comment 5 Matěj Laitl 2013-03-28 19:56:23 UTC
Anmol, basically my idea was to extend the "Transcode Tracks" dialog with the 
following radio button group (when transcoding to e.g. MP3 is selected):
(a) Transcode all tracks
(b) Transcode non-MP3 tracks and MP3 tracks with higher than selected bitrate
(c) Transcode only non-MP3 tracks
(d) Transcode only when needed for playability

where I consider (b) most useful, but it would need more infrastructure, so 
please skip this and implement only (a), (c) and (d) for now. Take care to 
implement saving of this preference along with other transcoding settings. The 
rest is to make actual transcoding implementation respect this setting, please 
try to avoid code duplication.

Please be aware that this is not exactly junior job and might take more time.
Comment 6 Anmol Ahuja 2013-03-28 20:08:38 UTC
https://git.reviewboard.kde.org/r/109781/

The patch isn't done yet, but here's how I was thinking of handling it:
I added a checkbox in the dialog, 
"Ignore files which're already the selected format"
which lets users skip transcoding files which are already in the selected output format.

Is this acceptable?
Comment 7 Matěj Laitl 2013-04-27 10:44:54 UTC
Git commit e84b44ebbb6c470af1272852ba8afd258aa858d0 by Matěj Laitl, on behalf of Anmol Ahuja.
Committed on 27/04/2013 at 12:36.
Pushed by laitl into branch 'master'.

Better and more configurable selection of tracks to transcode

FEATURES:
 * Allow to transcode only certain (different format, playability) tracks when
   transferring them to a collection; patch by Anmol Ahuja.

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

Anmol, I've made some very minor modifications, compare the diffs if you're
really curious. Thanks again for implementing this, it makes Amarok better.
FIXED-IN: 2.8
REVIEW: 109781
CCMAIL: Anmol Ahuja <darthcodus@gmail.com>
DIGEST: Amarok can now be configured not to transcode from e.g. MP3 to
MP3 and more. Patch by a newcomer Anmol Ahuja.

M  +2    -0    ChangeLog
M  +5    -4    src/core-impl/collections/db/sql/SqlCollectionLocation.cpp
M  +12   -8    src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp
M  +3    -2    src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.h
M  +3    -2    src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp
M  +2    -1    src/core-impl/collections/support/CollectionLocationDelegateImpl.h
M  +33   -11   src/core-impl/collections/umscollection/UmsCollectionLocation.cpp
M  +4    -2    src/core-impl/collections/umscollection/UmsCollectionLocation.h
M  +10   -3    src/core/collections/CollectionLocation.cpp
M  +3    -1    src/core/collections/CollectionLocationDelegate.h
M  +86   -8    src/core/transcoding/TranscodingConfiguration.cpp
M  +45   -5    src/core/transcoding/TranscodingConfiguration.h
M  +36   -2    src/transcoding/TranscodingAssistantDialog.cpp
M  +4    -1    src/transcoding/TranscodingAssistantDialog.h
M  +110  -68   src/transcoding/TranscodingAssistantDialog.ui
M  +2    -2    src/transcoding/TranscodingOptionsStackedWidget.cpp
M  +1    -1    src/transcoding/TranscodingOptionsStackedWidget.h
M  +31   -19   src/transcoding/TranscodingSelectConfigWidget.cpp
M  +4    -2    src/transcoding/TranscodingSelectConfigWidget.h
M  +2    -2    tests/core/collections/MockCollectionLocationDelegate.h

http://commits.kde.org/amarok/e84b44ebbb6c470af1272852ba8afd258aa858d0
Comment 8 Anmol Ahuja 2013-04-27 11:18:55 UTC
Thanks for the reviews and the commit :)
Comment 9 Thiago Jung Bauermann 2013-04-27 21:49:17 UTC
This is great news. Thanks Anmol Ahuja and Matěj Laitl!