Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

ImageMagick animators now can use extra_args#15739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

erelson
Copy link
Contributor

As a general solution for passing further args
to imagemagick when generating animations,
extra_args is now actually used. Extra args are
placed after all other args, but before the
output file name.

PR Summary

Note: This is a "general solution" to the problem I had.

My usecase: I found that if I passed a generated .gif back to Imagemagick, with the args-layers Optimize (more fully just:convert anim.gif -layers Optimize anim.gif), My several-megabyte gifs would be reduced in size by ~10x. In my current usage, I do this second pass via subprocess.

I then went ahead and modified my local copy of matplotlib (caveat: v1.5.1 with pyhon2.7 on ubuntu 14.04; but the code looks identical between master and 1.51).

With the fix in this PR, I can now doanim.save(gif_path, dpi=80, writer='imagemagick', extra_args=["-layers", "Optimize"]). This works fine giving visually identical animations, and further reduces file size another order of magnitude (in the one case I'm dealing with, I'm getting a resulting file 1/70th the size).

A concern I have (as I am not a regular imagemagick user, let alone an expert) is whether simply dumping extra args to image magick at the end of the hardcoded args is reasonable, or if order of operations is likely to make a mess with certain additional imagemagick arguments/operations... I might argue that any user adding imagemagick args probably knows what they're doing and would realize what's up. (I definitely benefited frommatplotlib.debug=helpful or something like that, which showed theconvert command that was actually invoked.) Alternately, maybethis is a spot where it's desirable to have more detailed documentation?

For reference, the code whereconvert is invoked with args looks like:

return ([self.bin_path(),                                           '-size', '%ix%i' % self.frame_size, '-depth', '8',         '-delay', str(self.delay), '-loop', '0',                   '%s:-' % self.frame_format]                               + self.output_args)

Therefore I am completely open to revising/redoing this PR and instead adding aImageMagick[File]Writer specific arg/property e.g.compress that explicitly adds-layers Optimize to the command (and leaveextra_args unused as it currently is).

Other notes:

  • I chose to modify the output_args property with additional logic, as this is similar to what is done with FFMpegBase and MencoderBase classes
    • Alternately, I probably would have just modified the_args() function instead.
  • I verified that both ImageMagickWriter and ImageMagickFileWriter classes use the extra args as expected

PR Checklist

  • Has Pytest style unit testsno similar tests exist for other classes usingextra_args
  • Code isFlake 8 compliantno additional warnings generated
  • New features are documented, with examples if plot related
  • [N/A] Documentation is sphinx and numpydoc compliant
  • [N/A] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

As a general solution for passing further argsto imagemagick when generating animations,extra_args is now actually used. Extra args areplaced after all other args, but before theoutput file name.
Copy link
Contributor

@anntzeranntzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

looks like it's fixing an oversight to me.

@erelson
Copy link
ContributorAuthor

Couple more thoughts:

  • oversight makes sense. In general it seemed like a lot of the animation class don't use everything in the base classes
  • I did observe that a.gif generated with this function found on stack overflow was not large like mine, though I have about 90 frames which might be more that typical.
  • with this fix, the benefit of using compression remains undocumented. This might be an argument in favor of making an explicit compress arg with a docstring.

Copy link
Contributor

@dopplershiftdopplershift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for the contribution. It also looks like an oversight to me, one driven by the fact that, as you noted, different tools have different argument handling and so there's no one universally safe way to add them. This looks good to me.

@dopplershift
Copy link
Contributor

I'm going to go ahead and merge this because it's a correct general fix. If you'd like to submit a PR to add an explicit compression option for ImageMagick, I'd be open to that too--but I don't want to hold this fix up for that.

@dopplershiftdopplershift merged commit0310b8a intomatplotlib:masterNov 21, 2019
@anntzeranntzer mentioned this pull requestNov 21, 2019
6 tasks
@QuLogicQuLogic added this to thev3.3.0 milestoneNov 22, 2019
@erelsonerelson deleted the imagemagick_extra_args branchNovember 27, 2019 04:46
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dopplershiftdopplershiftdopplershift approved these changes

@anntzeranntzeranntzer approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

4 participants
@erelson@dopplershift@anntzer@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp