From d8c6b568d134d8abc6b2f0c6ba0984270e73d0e7 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 10 Mar 2017 13:20:08 -0800 Subject: [PATCH 1/7] Round the output of adjusted_figsize to avoid floating point roundoff error --- lib/matplotlib/animation.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index fa03f90d7ad8..7d4f4092d1d0 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -23,6 +23,7 @@ import six from six.moves import xrange, zip +import numpy as np import os import platform import sys @@ -62,8 +63,8 @@ def adjusted_figsize(w, h, dpi, n): - wnew = int(w * dpi / n) * n / dpi - hnew = int(h * dpi / n) * n / dpi + wnew = np.round(int(w * dpi / n) * n / dpi) + hnew = np.round(int(h * dpi / n) * n / dpi) return wnew, hnew From feec74dbb7ab3904c5dfe58359d4ea23890020c7 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 10 Mar 2017 13:22:01 -0800 Subject: [PATCH 2/7] Adjust the frame size before writing to disk in case a library outside of matplotlib adjusted it in the meantime --- lib/matplotlib/animation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index 7d4f4092d1d0..3beed4be3542 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -338,6 +338,7 @@ def grab_frame(self, **savefig_kwargs): verbose.report('MovieWriter.grab_frame: Grabbing frame.', level='debug') try: + self._adjust_frame_size() # Tell the figure to save its data to the sink, using the # frame format and dpi. self.fig.savefig(self._frame_sink(), format=self.frame_format, From 30d5b6186d1168758652c39533fcdd9b2ae77deb Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 10 Mar 2017 15:25:08 -0800 Subject: [PATCH 3/7] use nextafter to correct floating point round-off error --- lib/matplotlib/animation.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index 3beed4be3542..3faf636dc48b 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -62,10 +62,19 @@ # how-to-encode-series-of-images-into-h264-using-x264-api-c-c ) +def correct_roundoff(x, dpi, n): + if int(x*dpi) % n != 0: + if int(np.nextafter(x, np.inf)*dpi) % n == 0: + x = np.nextafter(x, np.inf) + if int(np.nextafter(x, -np.inf)*dpi) % n == 0: + x = np.nextafter(x, -np.inf) + return x + + def adjusted_figsize(w, h, dpi, n): - wnew = np.round(int(w * dpi / n) * n / dpi) - hnew = np.round(int(h * dpi / n) * n / dpi) - return wnew, hnew + wnew = int(w * dpi / n) * n / dpi + hnew = int(h * dpi / n) * n / dpi + return (correct_roundoff(wnew, dpi, n), correct_roundoff(hnew, dpi, n)) # A registry for available MovieWriter classes From 08eb4347aacc48a856627720bc73350f4379e08a Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Sun, 12 Mar 2017 13:17:04 -0700 Subject: [PATCH 4/7] add test for movie writing floating point roundoff issues --- .travis.yml | 1 + lib/matplotlib/animation.py | 7 +++++-- lib/matplotlib/tests/test_animation.py | 11 ++++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 67333fe3b2a4..496f6f604a67 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ addons: packages: - inkscape - libav-tools + - libavcodec-extra-53 - gdb - mencoder - dvipng diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index 3faf636dc48b..4f94bdeb6936 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -288,8 +288,11 @@ def _adjust_frame_size(self): verbose.report('figure size (inches) has been adjusted ' 'from %s x %s to %s x %s' % (wo, ho, w, h), level='helpful') + else: + w, h = self.fig.get_size_inches() verbose.report('frame size in pixels is %s x %s' % self.frame_size, level='debug') + return w, h def setup(self, fig, outfile, dpi=None): ''' @@ -311,7 +314,7 @@ def setup(self, fig, outfile, dpi=None): if dpi is None: dpi = self.fig.dpi self.dpi = dpi - self._adjust_frame_size() + self._w, self._h = self._adjust_frame_size() # Run here so that grab_frame() can write the data to a pipe. This # eliminates the need for temp files. @@ -347,7 +350,7 @@ def grab_frame(self, **savefig_kwargs): verbose.report('MovieWriter.grab_frame: Grabbing frame.', level='debug') try: - self._adjust_frame_size() + self.fig.set_size_inches(self._w, self._h) # Tell the figure to save its data to the sink, using the # frame format and dpi. self.fig.savefig(self._frame_sink(), format=self.frame_format, diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py index 5f1d8a5ebf8c..d5aa7796ad99 100644 --- a/lib/matplotlib/tests/test_animation.py +++ b/lib/matplotlib/tests/test_animation.py @@ -145,6 +145,14 @@ def test_save_animation_smoketest(tmpdir, writer, extension): ax.set_xlim(0, 10) ax.set_ylim(-1, 1) + dpi = None + codec = None + if writer == 'ffmpeg': + # Issue #8253 + fig.set_size_inches((10.85, 9.2000000000000011)) + dpi = 100. + codec = 'h264' + def init(): line.set_data([], []) return line, @@ -160,7 +168,8 @@ def animate(i): with tmpdir.as_cwd(): anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5) try: - anim.save('movie.' + extension, fps=30, writer=writer, bitrate=500) + anim.save('movie.' + extension, fps=30, writer=writer, bitrate=500, + dpi=dpi, codec=codec) except UnicodeDecodeError: pytest.xfail("There can be errors in the numpy import stack, " "see issues #1891 and #2679") From 926a59fdb9cfbad05c9f2008ed8e828b5faa3a03 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 13 Mar 2017 11:34:04 -0500 Subject: [PATCH 5/7] don't attempt to use libav ffmpeg shim on ubuntu 12.04 --- .travis.yml | 1 - lib/matplotlib/animation.py | 31 ++++++++++++++++++++++--------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 496f6f604a67..67333fe3b2a4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,6 @@ addons: packages: - inkscape - libav-tools - - libavcodec-extra-53 - gdb - mencoder - dvipng diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index 4f94bdeb6936..700420b71dbb 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -400,16 +400,21 @@ def isAvailable(cls): if not bin_path: return False try: - p = subprocess.Popen(bin_path, - shell=False, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - creationflags=subprocess_creation_flags) - p.communicate() - return True + p = subprocess.Popen( + bin_path, + shell=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + creationflags=subprocess_creation_flags) + return cls._handle_subprocess(p) except OSError: return False + @classmethod + def _handle_subprocess(cls, process): + process.communicate() + return True + class FileMovieWriter(MovieWriter): '''`MovieWriter` for writing to individual files and stitching at the end. @@ -584,10 +589,18 @@ def output_args(self): return args + ['-y', self.outfile] + @classmethod + def _handle_subprocess(cls, process): + _, err = process.communicate() + # Ubuntu 12.04 ships a broken ffmpeg binary which we shouldn't use + if 'Libav' in err.decode(): + return False + return True + # Combine FFMpeg options with pipe-based writing @writers.register('ffmpeg') -class FFMpegWriter(MovieWriter, FFMpegBase): +class FFMpegWriter(FFMpegBase, MovieWriter): '''Pipe-based ffmpeg writer. Frames are streamed directly to ffmpeg via a pipe and written in a single @@ -608,7 +621,7 @@ def _args(self): # Combine FFMpeg options with temp file-based writing @writers.register('ffmpeg_file') -class FFMpegFileWriter(FileMovieWriter, FFMpegBase): +class FFMpegFileWriter(FFMpegBase, FileMovieWriter): '''File-based ffmpeg writer. Frames are written to temporary files on disk and then stitched From f287ac60316f79adf4d6774c4824c46622993772 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 15 Mar 2017 13:13:05 -0500 Subject: [PATCH 6/7] make test case a little clearer --- lib/matplotlib/animation.py | 2 +- lib/matplotlib/tests/test_animation.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index 700420b71dbb..d86fd45cb55f 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -66,7 +66,7 @@ def correct_roundoff(x, dpi, n): if int(x*dpi) % n != 0: if int(np.nextafter(x, np.inf)*dpi) % n == 0: x = np.nextafter(x, np.inf) - if int(np.nextafter(x, -np.inf)*dpi) % n == 0: + elif int(np.nextafter(x, -np.inf)*dpi) % n == 0: x = np.nextafter(x, -np.inf) return x diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py index d5aa7796ad99..017727016fe0 100644 --- a/lib/matplotlib/tests/test_animation.py +++ b/lib/matplotlib/tests/test_animation.py @@ -149,7 +149,7 @@ def test_save_animation_smoketest(tmpdir, writer, extension): codec = None if writer == 'ffmpeg': # Issue #8253 - fig.set_size_inches((10.85, 9.2000000000000011)) + fig.set_size_inches((10.85, 9.21)) dpi = 100. codec = 'h264' From 5d4f5deac7f4ab84e541f3dda5bb288b94622e9d Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Mon, 3 Apr 2017 15:15:11 -0400 Subject: [PATCH 7/7] DOC/MNT : add docs + make helper function private --- lib/matplotlib/animation.py | 39 +++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index d86fd45cb55f..03bc155c63a6 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -62,16 +62,36 @@ # how-to-encode-series-of-images-into-h264-using-x264-api-c-c ) -def correct_roundoff(x, dpi, n): - if int(x*dpi) % n != 0: - if int(np.nextafter(x, np.inf)*dpi) % n == 0: - x = np.nextafter(x, np.inf) - elif int(np.nextafter(x, -np.inf)*dpi) % n == 0: - x = np.nextafter(x, -np.inf) - return x +def adjusted_figsize(w, h, dpi, n): + '''Compute figure size so that pixels are a multiple of n + Parameters + ---------- + w, h : float + Size in inches + + dpi : float + The dpi + + n : int + The target multiple + + Returns + ------- + wnew, hnew : float + The new figure size in inches. + ''' + + # this maybe simplified if / when we adopt consistent rounding for + # pixel size across the whole library + def correct_roundoff(x, dpi, n): + if int(x*dpi) % n != 0: + if int(np.nextafter(x, np.inf)*dpi) % n == 0: + x = np.nextafter(x, np.inf) + elif int(np.nextafter(x, -np.inf)*dpi) % n == 0: + x = np.nextafter(x, -np.inf) + return x -def adjusted_figsize(w, h, dpi, n): wnew = int(w * dpi / n) * n / dpi hnew = int(h * dpi / n) * n / dpi return (correct_roundoff(wnew, dpi, n), correct_roundoff(hnew, dpi, n)) @@ -350,6 +370,9 @@ def grab_frame(self, **savefig_kwargs): verbose.report('MovieWriter.grab_frame: Grabbing frame.', level='debug') try: + # re-adjust the figure size in case it has been changed by the + # user. We must ensure that every frame is the same size or + # the movie will not save correctly. self.fig.set_size_inches(self._w, self._h) # Tell the figure to save its data to the sink, using the # frame format and dpi.