API: forbid unsafe savefig kwargs to AbstractMovieWriter.grab_frame

[tacaswell]

PR Summary

closes #25608

Per #25608 (comment) I do not think there is a way to make these keywords work in the context of a saved animation.

PR Checklist

Documentation and Tests

• Has pytest style unit tests (and pytest passes)
• Documentation is sphinx and numpydoc compliant (the docs should build without error).
• [na] New plotting related features are documented with examples.

Release Notes

• [na] New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
• API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
• Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

[rcomer]

Completely out of my wheelhouse here, but the comment in the matplotlibrc seems to suggest that bbox_inches='tight' should work for at least some writers.

matplotlib/lib/matplotlib/mpl-data/matplotlibrc

Lines 685 to 688 in 6686f97

	#savefig.bbox: standard # {tight, standard}
	# 'tight' is incompatible with pipe-based animation
	# backends (e.g. 'ffmpeg') but will work with those
	# based on temporary files (e.g. 'ffmpeg_file')

Also if you set bbox_inches to a specific BBox, would it not produced the same size image each frame?

[tacaswell]

After going down a far deeper rabbit hole than I expected (that involved trying and failin gto get a mpeg4 decoder installed on fedora) to see what ffmpeg -i ... does with input files that vary in size.

import matplotlib.pyplot as plt

fig, ax = plt.subplots()

for j in range(100):
    ax.set_title(f"frame {j}")
    fig.set_size_inches(j % 10 + 2, j % 10 + 2)
    fig.savefig(f"/tmp/mv/frame-{j:04d}.png")

ffmpeg -i 'frame-%04d.png' -vcodec av1 test.mp4

It seems to work (but I would call the output more artistic than informative). I think it is scaling the subsequent files to match the size of the first frame so the axes move / resize frame to frame.

We already do some checking in Aimation.save to protect the user from this sort of thing going wrong:

matplotlib/lib/matplotlib/animation.py

Lines 1042 to 1057 in 61ed3f4

	if 'bbox_inches' in savefig_kwargs:
	_log.warning("Warning: discarding the 'bbox_inches' argument in "
	"'savefig_kwargs' as it may cause frame size "
	"to vary, which is inappropriate for animation.")
	savefig_kwargs.pop('bbox_inches')
	# Create a new sequence of frames for saved data. This is different
	# from new_frame_seq() to give the ability to save 'live' generated
	# frame information to be saved later.
	# TODO: Right now, after closing the figure, saving a movie won't work
	# since GUI widgets are gone. Either need to remove extra code to
	# allow for this non-existent use case or find a way to make it work.
	if mpl.rcParams['savefig.bbox'] == 'tight':
	_log.info("Disabling savefig.bbox = 'tight', as it may cause "
	"frame size to vary, which is inappropriate for "
	"animation.")

We talked about this on the call today and the consensus is that while it may "work" with the file based writers, it is likely not what a user wants, if the user wants to save varying size pngs they can still do so (and will have to call ffmpeg them selves), and because this is a lower-level API than Animation.save it makes sense to be very strict an raise (rather than just warning).

I also think there is a consistency arguement that if we prevent something with some writers (because it silently produces totally broken output) we should prevent the same thing in other writers (where it would silently produces silly output).

[tacaswell]

@rcomer I updated the comment in the rcparams.

[QuLogic]

Side note, but I don't know what this comment is about.