Bug 300551 - amarok passes invalid option -n to libav's ffmpeg
Summary: amarok passes invalid option -n to libav's ffmpeg
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Transcoding (show other bugs)
Version: 2.5-git
Platform: unspecified Linux
: NOR normal
Target Milestone: 2.6
Assignee: Amarok Developers
URL:
Keywords: release_blocker
Depends on:
Blocks:
 
Reported: 2012-05-24 10:54 UTC by Piotr Keplicz
Modified: 2012-05-26 16:07 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.6
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Keplicz 2012-05-24 10:54:33 UTC
When transcoding tracks, amarok adds "-n" option to the ffmpeg call. This causes transcoding job to fail when using a ffmpeg coming from the libav-tools:

amarok: ffmpeg: ffmpeg version 0.8.1-4:0.8.1-1really0u1, Copyright (c) 2000-2011 the Libav developers 
amarok: ffmpeg:   built on Mar 22 2012 20:42:48 with gcc 4.6.1 
amarok: ffmpeg: This program is not developed anymore and is only provided for compatibility. Use avconv instead (see Changelog for the list of incompatible changes). 
amarok: ffmpeg:  
amarok: ffmpeg: Unrecognized option 'n' 
amarok: ffmpeg: Failed to set value '-i' for option 'n' 
amarok: ffmpeg:  
amarok: NAY, transcoding fail! 

Reproducible: Always

Steps to Reproduce:
1. Install the libav-tools instead of the traditional ffmpeg.
2. Attempt to transcode a track.
Actual Results:  
Transcoding fails without a meaningful message ("copying tracks failed").

Expected Results:  
Amarok copies transcoded tracks.

Using amarok from git, 2012-05-23 on a Ubuntu 12.04. This will most probably affect all the distros switching from traditional ffmpeg to libav.

A trivial proof-of-concept patch fixes the problem here:

diff --git a/src/transcoding/TranscodingJob.cpp b/src/transcoding/TranscodingJob.cpp
index fba1446..24552dc 100644
--- a/src/transcoding/TranscodingJob.cpp
+++ b/src/transcoding/TranscodingJob.cpp
@@ -67,7 +67,7 @@ Job::init()
     //First the executable...
     m_transcoder->setProgram( "ffmpeg" );
-      //... no not overwrite output files but exit if file exists, otherwise ffmpeg is interactive and hangs
-    *m_transcoder << QString( "-n" );
     //... then we'd have the infile configuration followed by "-i" and the infile path...
     *m_transcoder << QString( "-i" )
                   << m_src.path();

Obviously amarok could try to use avconv directly and fall back to ffmpeg when necessary.
Comment 1 Matěj Laitl 2012-05-24 11:00:16 UTC
Gosh, another ffmpeg incopatibility. The lines
//... no not overwrite output files but exit if file exists, otherwise ffmpeg is interactive and hangs
*m_transcoder << QString( "-n" );

are there for purpose! Piotr, is there another option to make libav's ffmpeg non-interactive? (especially when the target file already exists)
Comment 2 Piotr Keplicz 2012-05-24 11:13:04 UTC
That would be:
-y  Overwrite output files.
Comment 3 Matěj Laitl 2012-05-24 11:15:33 UTC
(In reply to comment #2)
> That would be: -y  Overwrite output files.

Hmm, we probably don't want this, but at least something. There's no equivalent of the -n option that does the contrary?
Comment 4 Piotr Keplicz 2012-05-24 11:32:52 UTC
No, there isn't: http://libav.org/ffmpeg.html#Main-options
I suppose -n was added to ffmpeg only recently. Indeed avconv/ffmpeg drops to interactive mode, when the output file exists:

$ ffmpeg -i 01.m4a 01.mp3
[...]
File '01.mp3' already exists. Overwrite ? [y/N]
Comment 5 Matěj Laitl 2012-05-25 20:39:15 UTC
We don't want to release 2.6 wihout fix to this bug. I'll have a look at it after beta tagging.
Comment 6 Matěj Laitl 2012-05-26 16:07:57 UTC
Git commit b1dfac7932bc89127b4ea1098a143efe9e068f99 by Matěj Laitl.
Committed on 26/05/2012 at 18:05.
Pushed by laitl into branch 'master'.

TranscodingJob: don't pass -n to ffmpeg, check file existence manually

BUGFIXES:
 * Transcoding: fix compatibility with libav's ffmpeg
FIXED-IN: 2.6
DIGEST: Bugfix

M  +1    -0    ChangeLog
M  +19   -5    src/transcoding/TranscodingJob.cpp
M  +5    -1    src/transcoding/TranscodingJob.h

http://commits.kde.org/amarok/b1dfac7932bc89127b4ea1098a143efe9e068f99