Opened 8 years ago

Closed 8 years ago

#11650 closed defect (fixed)

Make 'convert' the standard way to produce animated gifs (again)

Reported by: jhpalmieri Owned by: jason, was
Priority: major Milestone: sage-4.8
Component: graphics Keywords: animate convert ffmpeg
Cc: Merged in: sage-4.8.alpha3
Authors: John Palmieri Reviewers: Dan Drake
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11170 Stopgaps:

Description

In ticket #11170, code was added so that if ffmpeg is present, it gets used by default to produce animated gifs. These can be much larger than the ones produced by 'convert', plus there may be other problems with ffmpeg. The attached patch makes 'convert' the default again.

Attachments (1)

trac_11650-animate.patch (5.9 KB) - added by jhpalmieri 8 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by jhpalmieri

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by kini

  • Dependencies set to 11170

Just by the by, what do you think about bundling GraphicsMagick? (a fork of ImageMagick? by a group of its original developers) with Sage? Apparently it is faster than ImageMagick? (I'm told, by a user, that it is faster by a factor of three for these animation type things). It's also X11-licensed, so if I understand correctly it is possible to bundle it, unlike ImageMagick?. The source occupies ~38 MB.

comment:3 Changed 8 years ago by kini

  • Dependencies changed from 11170 to #11170

comment:4 Changed 8 years ago by jhpalmieri

I've never used GraphicsMagick, but I'll take a look some time. It would be nice to include it, if it's that small. Does it need libjpeg or anything else like that?

comment:5 Changed 8 years ago by ddrake

  • Status changed from needs_review to needs_work

This looks good. Two notes:

  • You can't pass use_ffmpeg=True when using .save(). If it's easy to do, can .save() pass that onto .gif()? Or at least change the docstring in .save() to tell users to go directly to .gif() if they want to use ffmpeg.
  • Somehow, the delay parameter is now a string:
    sage: a.gif('bar2.gif', use_ffmpeg=True)
    ---------------------------------------------------------------------------
    TypeError                                 Traceback (most recent call last)
    
    /home/drake/s/sage-4.7.2.alpha2/<ipython console> in <module>()
    
    /home/drake/s/sage-4.7.2.alpha2/local/lib/python2.6/site-packages/sage/plot/animate.pyc in gif(self, delay, savefile, iterations, show_path, use_ffmpeg)
        361                 self.ffmpeg(savefile=savefile, show_path=show_path,
        362                             output_format='gif', delay=delay,
    --> 363                             iterations=iterations)
        364             else:
        365                 if not have_convert:
    
    /home/drake/s/sage-4.7.2.alpha2/local/lib/python2.6/site-packages/sage/plot/animate.pyc in ffmpeg(self, savefile, show_path, output_format, ffmpeg_options, delay, iterations, verbose)
        566                 ffmpeg_options += ' -pix_fmt rgb24 -loop_output %s ' % iterations
        567             if delay is not None and output_format != 'mpeg' and output_format != 'mpg':
    --> 568                 early_options += ' -r %s -g 3 ' % int(100/delay)
        569             savefile = os.path.abspath(savefile)
        570             pngdir = self.png()
    
    TypeError: unsupported operand type(s) for /: 'int' and 'str'
    

Calling .ffmpeg() without the patch here works fine.

comment:6 Changed 8 years ago by jhpalmieri

  • Status changed from needs_work to needs_review

Here's a new patch; the only changes are the following:

  • sage/plot/animate.py

    diff --git a/sage/plot/animate.py b/sage/plot/animate.py
    a b please install it and try again.""" 
    587587                print "Error running ffmpeg."
    588588                raise
    589589
    590     def save(self, filename=None, show_path=False):
     590    def save(self, filename=None, show_path=False, use_ffmpeg=False):
    591591        """
    592592        Save this animation.
    593593
    please install it and try again.""" 
    598598        -  ``show_path`` - boolean (default: False); if True,
    599599           print the path to the saved file
    600600
     601        - ``use_ffmpeg`` - boolean (default: False); if True, use
     602          'ffmpeg' by default instead of 'convert' when creating GIF
     603          files.
     604
    601605        If filename is None, then in notebook mode, display the
    602606        animation; otherwise, save the animation to a GIF file.  If
    603607        filename ends in '.sobj', save to an sobj file.  Otherwise,
    please install it and try again.""" 
    632636                suffix = '.gif'
    633637
    634638        if filename is None or suffix == '.gif':
    635             self.gif(savefile=filename, show_path=show_path)
     639            self.gif(savefile=filename, show_path=show_path,
     640                     use_ffmpeg=use_ffmpeg)
    636641            return
    637642        elif suffix == '.sobj':
    638643            SageObject.save(self, filename)

This allows you to do a.save('foobar.gif', use_ffmpeg=True).

I'm not seeing the issue with delay being a string. Can you please try again?

Changed 8 years ago by jhpalmieri

comment:7 Changed 8 years ago by ddrake

  • Reviewers set to Dan Drake
  • Status changed from needs_review to positive_review

Now this looks good. Passes all doctests and works properly. I don't know what happened with the delay option; it now works.

comment:8 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.