add an ffmpeg option to the animate command

[jhpalmieri]

The attached patch allows you to do this:

sage: a = animate([sin(x + float(k)) for k in srange(0,2*pi,0.7)])
sage: a.ffmpeg('output.mpg')

or

sage: a.ffmpeg('output.avi')

Also, calling

sage: a.save('output.mpg')  # or a.save('output.avi')

for example will work, calling the ffmpeg method. If ffmpeg is installed, it is also used to construct animated gifs, since it's faster than convert.

There was some discussion of this here: https://groups.google.com/d/topic/sage-support/TdQ29S32K9k/discussion.

This also clean up some doctest flags, using # optional -- ImageMagick and # optional -- ffmpeg to mark the appropriate tests. For me, tests pass with each of the following combinations, and there are no stray graphics files created:

sage -t -long -optional animate.py    (with convert and ffmpeg installed)
sage -t -long -only-optional=imagemagick (with only convert installed)
sage -t -long -only-optional=ffmpeg (with only ffmpeg installed)
sage -t -long

CC: @sagetrac-mhampton @novoselt

Component: graphics

Author: John Palmieri

Reviewer: David Kirkby, Dan Drake

Merged: sage-4.7.1.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/11170

[jhpalmieri]

comment:2

This should also fix the problem at #10655.

[sagetrac-drkirkby]

comment:3

Would it be sensible to use cksum to create a checksum of the files, so we know if the expected file is generated? Since otherwise I don't think we know if this is actually producing a decent animation That might cause an issue if the version of the external software is changed. Differnet versions might change the algorithm, or add the string of the version in some way, so this might not work. (cksum is portable - unlike any attempt to get an md5 checksum will be.) Another option would be to check the file is of the correct size. If one measured it in KB, for example with:

drkirkby@laptop:~$ du -k testcc.sh
4	testcc.sh

then I doubt the size of the file would change much from version to version. Note we would need the -k option on du. Whilst that is the default on Linux, POSIX states the output should be in blocks of 512 bytes, but Linux ignores this, and by default prints the output in block of 1024 bytes. This is one of the things that makes Linux non-POSIX compatible)

I think it would be more useful to suggest checking for programs like conjure or *mogrify *, as anyone could have a program called convert on their system, which is unrelated to ImageMagick, since the word is in such common use. One can easily imagine something having something to convert between different units (mm, inches etc) and calling the program convert.

I think somewhere, at some point, we should try to document what packages would be useful to have with Sage. We say LaTex is highly recommended, but it would be worth documenting things that are useful like FFmpeg and ImageMagick. But that of course is for another ticket.

Dave

[nilesjohnson]

comment:4

Nice! Here are a few things I noticed in the patch:

line 334: "neither ffmpeg or ImageMagick is not installed...": needs to be cleaned up

line 458: "Returns an mpeg showing...": this is from the docstring for .ffmpeg -- should it be reworded to include other video formats?

line 527: "saving an animation to an mpg file..." : this error message is shown even if another format is chosen -- could be confusing if a user misunderstands and thinks that mpg format is the underlying problem.

[jhpalmieri]

comment:5

Niles: thanks, I think I've fixed those now.

Dave: since I would guess that the size of the animation may change from one version of ImageMagick to another, and the same for ffmpeg, I don't see how to use checksums effectively.

Also, we actually don't check for the presence of convert, we just run it. If it doesn't exit with zero status (as should happen if it's not there, or if it gets called with bad arguments -- I hope this is what would happen if there were another program called "convert" lying around and we call "convert -delay ... -loop ... *.png output.gif"), we raise an error. We could also check for the presence of a directory /usr/lib/ImageMagick-.../ or /usr/local/lib/ImageMagick-.../ or something like that. On my machine, I don't actually see any libraries in that directory, just some xml files for configuration. On sage.math, there are actual libraries in some of the subdirectories of /usr/lib/ImageMagick-6.3.7/.

I'm not sure the best way to deal with this. Since I haven't heard complaints about our current method for using "convert", and since this patch doesn't change that, I would be inclined to leave it as is.

Meanwhile, recommending the installation of ffmpeg or ImageMagick seems like a good idea, and so I've added it to the patch.

[sagetrac-drkirkby]

comment:6

Replying to @jhpalmieri:

Niles: thanks, I think I've fixed those now.

Dave: since I would guess that the size of the animation may change from one version of ImageMagick to another, and the same for ffmpeg, I don't see how to use checksums effectively.

My point was that even if the exact details of the animation changed from version to version, I would not expect the size to vary much. So perhaps checking the number of KB (or even MB of the file) would give a reasonable confidence the program is producing an mpeg file of about the right length.

Also, we actually don't check for the presence of convert, we just run it. If it doesn't exit with zero status (as should happen if it's not there, or if it gets called with bad arguments -- I hope this is what would happen if there were another program called "convert" lying around and we call "convert -delay ... -loop ... *.png output.gif"), we raise an error.

Yes agreed, any sensible program would exit with an error in this case. Just in general I think making use of a more obscure file would have been preferable. But I don't want to drag the ticket on by making changes outside of what you are intending. (You know that really winds me up some times!)

We could also check for the presence of a directory /usr/lib/ImageMagick-.../ or /usr/local/lib/ImageMagick-.../ or something like that.

That would be quite platform dependent. It would not work on some systems like AIX, where such files are likely to be under /pware. Neither would it work if someone wanted to install ImageMagick, but did not have root access, so had to build it under their home directory.

On my machine, I don't actually see any libraries in that directory, just some xml files for configuration. On sage.math, there are actual libraries in some of the subdirectories of /usr/lib/ImageMagick-6.3.7/.

I feel anything like this is a bit risky.

I'm not sure the best way to deal with this. Since I haven't heard complaints about our current method for using "convert", and since this patch doesn't change that, I would be inclined to leave it as is.

Fair enough. Perhaps for another ticket, we could consider making use of one of the more obscure program names in ImageMagick for a test.

Meanwhile, recommending the installation of ffmpeg or ImageMagick seems like a good idea, and so I've added it to the patch.

Good. I think we really need a list of such programs (outside the scope of this ticket). I'll raise that on sage-devel. If we can get a list, these can be added to the installation guide.

As you are aware, I'm not a Python guru, so don't feel able to give this a proper review, but there was something else that I found a bit odd. I don't know if there is any standard for specifying if optional tests get run. Take a look at for one example for Mathematica

    sage: mobj = mathematica(x^2-1)             # optional - mathematica

in the file

./devel/sage-main/sage/interfaces/mathematica.py

and now compare that to yours on this ticket.

sage: a.show() # optional -- ImageMagick 

Two obvious things are different can be seen.

• The Mathematica example uses two hypens, your example uses one.
• The Mathematica example uses all lower case, despite Wolfram Research use an upper case M at the start. Your example uses the proper case for ImageMagick

IF this means that one needs to specify how to run the Mathematica and ImageMagick tests differently, then I think that's a bad idea. But I've got no idea what is considered "right" in sage, but I think we should at least be consistent.

I suspect the most robust solution is for the code to accept either case - i.e. convert all to lower case, then accept that. Then it would not matter what the user typed. Again outside this ticket, but if it has been agreed to use all lower case, then perhaps you should use that).

It does rather annoy me Sage refers to Matlab, despite MATLAB (with all capital letters) is a registered trademark, I actually wish we would stick to how the upstream developers use the term.

Dave

[sagetrac-drkirkby]

Reviewer: David Kirkby

[jhpalmieri]

comment:8

Replying to @sagetrac-drkirkby:

My point was that even if the exact details of the animation changed from version to version, I would not expect the size to vary much. So perhaps checking the number of KB (or even MB of the file) would give a reasonable confidence the program is producing an mpeg file of about the right length.

For either ffmpeg or ImageMagick by themselves, this might be right, but ffmpeg produces GIFs which are larger (by a fair amount) than ImageMagick does, it seems to me in my limited testing.

Two obvious things are different can be seen.

• The Mathematica example uses two hypens, your example uses one.
• The Mathematica example uses all lower case, despite Wolfram Research use an upper case M at the start. Your example uses the proper case for ImageMagick

IF this means that one needs to specify how to run the Mathematica and ImageMagick tests differently, then I think that's a bad idea. But I've got no idea what is considered "right" in sage, but I think we should at least be consistent.

I suspect the most robust solution is for the code to accept either case - i.e. convert all to lower case, then accept that. Then it would not matter what the user typed. Again outside this ticket, but if it has been agreed to use all lower case, then perhaps you should use that).

The script sage-doctest (which is written in python -- sorry) strips all hyphens from the comments after each doctest, and converts both the comment and the command-line argument to lower case, so this is not a problem. I remember fixing this problem a while ago, and I just double-checked the script to be sure. Also, running with -only-optional=imagemagick runs the doctests, despite the difference in case.

[jhpalmieri]

comment:11

Attachment: trac_11170-animate.patch.gz

Replying to @dandrake:

This looks very good, and I'm excited to get it in. I do see one tiny problem: in the ffmpeg() function, line 524, you import have_program but don't use it -- that's the job of self._have_ffmpeg(). So I think you can delete line 524.

Done -- here's a new patch. I also rebased it against 4.7.rc1 -- there were some changes to the same parts of the installation guide, so the old version wasn't applying cleanly.

[jdemeyer]

Merged: sage-4.7.1.alpha1

[kini]

comment:14

I'm not sure that it's a good idea to make ffmpeg the default animation method. The code I posted recently on #11313 results in a 835Kb .gif file with the ImageMagick method, but a 40MB .gif from ffmpeg! This would seem to bode ill for sagenb.org's bandwidth... Not to mention the ffmpeg animation breaks the aspect ratio.

[jhpalmieri]

comment:15

I think you're right. We should open another ticket and fix this.

[jhpalmieri]

comment:16

See #11650.