Bug fixes for gnupod.

gnupod is broken with FFmpeg within the last 2 years and the ImageMagick
check is broken because it returns with an error return of 1 instead of 0.


From d9001005771fab59c8e6024f8beb5d7a62ceebcc Mon Sep 17 00:00:00 2001
From: Brad Smith 
Date: Mon, 30 May 2011 16:34:31 -0400
Subject: [PATCH] Fix commandline options used for modern FFmpeg and fix
 ImageMagick test within autoconf script.

---
 configure.ac               |    4 ++--
 src/gnupod_convert_FLAC.pl |    2 +-
 src/gnupod_convert_RIFF.pl |    5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index c7811df..aa2bd99 100755
--- a/configure.ac
+++ b/configure.ac
@@ -130,7 +130,7 @@ printf "%s\n" " done" 
 printf "%s" "checking for ffmpeg with AAC support... "
-ffmpeg -formats 2>&1|egrep " aac" > /dev/null
+ffmpeg -codecs 2>&1|egrep " aac" > /dev/null
  if test $? = 0; then
   printf "%s\n" "found!"
   ENCS="mpeg4 $ENCS"
@@ -142,7 +142,7 @@ ffmpeg -formats 2>&1|egrep " aac" > /dev/null printf "%s" "checking for ImageMagick..."
 convert --version > /dev/null 2>&1
- if test $? = 0; then
+ if test $? = 1; then
   printf "%s\n" "found!"
   IMAGIC="Yes"
  else
diff --git a/src/gnupod_convert_FLAC.pl b/src/gnupod_convert_FLAC.pl
index 210ff59..e9bd83b 100644
--- a/src/gnupod_convert_FLAC.pl
+++ b/src/gnupod_convert_FLAC.pl
@@ -122,7 +122,7 @@ else {
 # Check if ffmpeg knows how to encode 'alac'
 sub check_ffmpeg_alac {
 	my @alac_support = grep(/\s+DEA\s+alac/,split(/\n/,
-		`ffmpeg -formats 2> /dev/null`));
+		`ffmpeg -codecs 2> /dev/null`));
 	return (defined(@alac_support));
 }diff --git a/src/gnupod_convert_RIFF.pl b/src/gnupod_convert_RIFF.pl
index c4d2fbc..32a5291 100644
--- a/src/gnupod_convert_RIFF.pl
+++ b/src/gnupod_convert_RIFF.pl
@@ -45,7 +45,8 @@ elsif($gimme eq "GET_VIDEO") {
 	
 	my $x = system("ffmpeg", "-i", $file, "-acodec", $acodec, "-ab", "128k", "-vcodec", "mpeg4",
 	               "-b", "1200kb", "-mbd", 2, "-flags", "+4mv+trell", "-aic", 2, "-cmp", 2,
-	               "-subcmp", 2, "-s", "320x240", "-r", "29.97", $tmpout);
+	               "-subcmp", 2, "-s", "320x240", "-r", "29.97", "-strict", "experimental",
+	               $tmpout);
 	print "PATH:$tmpout\n";
 }
 else {
@@ -58,7 +59,7 @@ else {
 # still shall call it with AAC
 sub check_ffmpeg_aac {
 	my @newstyle = grep(/\s+EA\s+libfaac/,split(/\n/,
-	               `ffmpeg -formats 2> /dev/null`));
+	               `ffmpeg -codecs 2> /dev/null`));
 	return (defined(@newstyle) ? 'libfaac' : 'aac');
 }-- 
1.7.5-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Hello Brad,

Thanks for the ffmpeg fixes, they look very useful.On 30-5-11 22:39 , Brad wrote:
>  printf "%s" "checking for ImageMagick..."
>  convert --version > /dev/null 2>&1
> - if test $? = 0; then
> + if test $? = 1; thenThis change is incorrect. Convert returns 0 when it exists. A non-zero
result is expected when it does not exist.

$ convert --version
Version: ImageMagick 6.6.4-0 2010-09-18 Q16 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2010 ImageMagick Studio LLC
Features: OpenMP

$ echo $?
0
$ notthere --version
-bash: notthere: command not found
$ echo $?
127

However, the check in configure is informative only. The result is not
used in the perl code of gnupod.

Regards,

Richard

----- Original message -----
> Hello Brad,
> 
> Thanks for the ffmpeg fixes, they look very useful.
> 
> On 30-5-11 22:39 , Brad wrote:
> > printf "%s" "checking for ImageMagick..."
> > convert --version > /dev/null 2>&1
> > - if test $? = 0; then
> > + if test $? = 1; then
> 
> This change is incorrect. Convert returns 0 when it exists. A non-zero
> result is expected when it does not exist.
> 
> $ convert --version
> Version: ImageMagick 6.6.4-0 2010-09-18 Q16 http://www.imagemagick.org
> Copyright: Copyright (C) 1999-2010 ImageMagick Studio LLC
> Features: OpenMP
> 
> $ echo $?
> 0
> $ notthere --version
> -bash: notthere: command not found
> $ echo $?
> 127
> 
> However, the check in configure is informative only. The result is not
> used in the perl code of gnupod.I'll have to check the version I am using when I get home but it returns 1 instead of 0. I changed it specifically because the test was failing to detect the presence of ImageMagick even though its there.-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

On 31/05/11 2:55 PM, Richard van den Berg wrote:
> Hello Brad,
>
> Thanks for the ffmpeg fixes, they look very useful.
>
> On 30-5-11 22:39 , Brad wrote:
>>   printf "%s" "checking for ImageMagick..."
>>   convert --version>  /dev/null 2>&1
>> - if test $? = 0; then
>> + if test $? = 1; then
>
> This change is incorrect. Convert returns 0 when it exists. A non-zero
> result is expected when it does not exist.
>
> $ convert --version
> Version: ImageMagick 6.6.4-0 2010-09-18 Q16 http://www.imagemagick.org
> Copyright: Copyright (C) 1999-2010 ImageMagick Studio LLC
> Features: OpenMP
>
> $ echo $?
> 0
> $ notthere --version
> -bash: notthere: command not found
> $ echo $?
> 127
>
> However, the check in configure is informative only. The result is not
> used in the perl code of gnupod.$ convert --version
Version: ImageMagick 6.4.5 2011-05-21 Q16 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2008 ImageMagick Studio LLC

$ echo $?
1

Anyway, I'll keep that part as a local change for our OpenBSD port just
so developers and so on are not confused about the autoconf script not
finding ImageMagick even though it is there.-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Hi Brad & Richard,On Tue, May 31, 2011 at 06:11:10PM -0400, Brad wrote:
> On 31/05/11 2:55 PM, Richard van den Berg wrote:
> >Hello Brad,
> >
> >Thanks for the ffmpeg fixes, they look very useful.@Richard/Brad: Do you happen to know when ffmpeg changed their output?
It's not like gnupod was wrong using "-formats". I just checked a
debian lenny and there "-codecs" yields an error:
=============
$ ffmpeg -codecs
FFmpeg version SVN-r13582, Copyright (c) 2000-2008 Fabrice Bellard, et al.
  configuration: --prefix=/usr --libdir=${prefix}/lib
--shlibdir=${prefix}/lib --bindir=${prefix}/bin
--incdir=${prefix}/include/ffmpeg --enable-shared --enable-libmp3lame
--enable-gpl --enable-libfaad --mandir=${prefix}/share/man
--enable-libvorbis --enable-pthreads --enable-libfaac --enable-libxvid
--enable-postproc --enable-libamr-nb --enable-libamr-wb --enable-x11grab
--enable-libgsm --enable-libx264 --enable-liba52 --enable-libtheora
--extra-cflags=-Wall -g -fPIC -DPIC --cc=ccache cc --enable-swscale
--enable-libdc1394 --enable-nonfree --disable-mmx --disable-stripping
--enable-avfilter --enable-libdirac --disable-decoder=libdirac
--enable-libschroedinger --disable-encoder=libschroedinger --disable-altivec
--disable-armv5te --disable-armv6 --disable-vis
  libavutil version: 49.7.0
  libavcodec version: 51.58.0
  libavformat version: 52.16.0
  libavdevice version: 52.0.0
  libavfilter version: 0.0.0
  built on May  3 2009 12:02:42, gcc: 4.3.2
ffmpeg: missing argument for option '-codecs'
==============

It seems we might have to support both "-formats" and "-codecs" if we want
to keep support for older systems alive.> >On 30-5-11 22:39 , Brad wrote:
> >>  printf "%s" "checking for ImageMagick..."
> >>  convert --version>  /dev/null 2>&1
> >>- if test $? = 0; then
> >>+ if test $? = 1; then
> >I think we've seen that issue before ... let me google that
for you :-)

http://lmgtfy.com/?q=gnupod+convert+exit+code

I never heard back from chris or the ImageMagick developers
so it remains a mystery why they changed the exit code.
But as Richard mentioned below, the 6.6 version (just checked
the one on ubuntu 11.04 and debian squeeze) seem to have reverted
to the old behaviour:
=====
hlangos@T60:~$ convert -version
Version: ImageMagick 6.6.2-6 2011-03-16 Q16 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2010 ImageMagick Studio LLC
Features: OpenMP 

hlangos@T60:~$ echo $?
0
=====
henrik@www:~$ convert -version
Version: ImageMagick 6.6.0-4 2010-11-16 Q16 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2010 ImageMagick Studio LLC
Features: OpenMP 

henrik@www:~$ echo $?
0
=====

So I guess 6.4 was a glitch.> Anyway, I'll keep that part as a local change for our OpenBSD port just
> so developers and so on are not confused about the autoconf script not
> finding ImageMagick even though it is there.If you implement changes in your OpenBSD port I suggest you
set yourself a flag to change it back when you upgrade to a newer
ImageMagick version.

BTW: If you have suggestions to improve the portability of gnupod, please
feel free to contact us. I have been busy (more than usual) in the last half
year, but I hope I'll get around to rewrite the backend to support more
ipod models ... eventually.

cheers
-henrik

On 31/05/11 7:34 PM, H. Langos wrote:
> Hi Brad&  Richard,
>
> On Tue, May 31, 2011 at 06:11:10PM -0400, Brad wrote:
>> On 31/05/11 2:55 PM, Richard van den Berg wrote:
>>> Hello Brad,
>>>
>>> Thanks for the ffmpeg fixes, they look very useful.
>
> @Richard/Brad: Do you happen to know when ffmpeg changed their output?
> It's not like gnupod was wrong using "-formats". I just checked a
> debian lenny and there "-codecs" yields an error:It was "wrong" using -formats with newer FFmpeg if you want the checks 
within the autoconf script and Perl code to actually work as expected.
-formats used to list codecs and now you have to use -codecs to list
the codecs. Since Nov 19th 2009.

http://git.videolan.org/?p=ffmpeg.git;a=commi...

Anyway, I was just making an attempt to push this stuff back upstream 
but it looks like as usual with FFmpeg (and more so with the command 
line tools) its a pain to support a wide range of versions. But long
term something has to be done since OS's are not going backwards with
their FFmpeg releases/snaps ;)-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.