Skip to content

FIX: ffmpeg logging level #10739

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Mar 12, 2018
Merged

FIX: ffmpeg logging level #10739

merged 3 commits into from
Mar 12, 2018

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Mar 9, 2018

PR Summary

Apparently closes #10729 (thanks @cgohlke and @anntzer )

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@jklymak jklymak added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Mar 9, 2018
@jklymak jklymak added this to the v2.2.1 milestone Mar 9, 2018
@anntzer
Copy link
Contributor

anntzer commented Mar 9, 2018

I would add at least a big fat comment warning "at higher log levels, this will crash after too many frames". Or just disable logging altogether there, depends on your POV. Or move it to e.g. log.getEffectiveLevel() < logging.DEBUG - 1 so the user really needs to set a loglevel even below DEBUG.

@jklymak
Copy link
Member Author

jklymak commented Mar 9, 2018

I think the former - users should be able to see the ffmpeg debug info if they want. Hopefully not too many debug on 1000-fram animations...

@jklymak jklymak force-pushed the fix-ffmpeg-log-level branch from a449c39 to 91654d2 Compare March 9, 2018 17:22
anntzer
anntzer approved these changes Mar 9, 2018
View reviewed changes
Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

at least it's an improvement...

@jklymak jklymak force-pushed the fix-ffmpeg-log-level branch from 91654d2 to a65b9c1 Compare March 9, 2018 17:25
ImportanceOfBeingErnest
ImportanceOfBeingErnest reviewed Mar 10, 2018
View reviewed changes
lib/matplotlib/axes/_axes.py Outdated
@@ -2218,7 +2218,7 @@ def bar(self, *args, **kwargs):
if yerr is not None:
yerr = self.convert_yunits(yerr)

x, height, width, y, linewidth = np.broadcast_arrays(
x, height, width, y, ll = np.broadcast_arrays(
Copy link
Member

Choose a reason for hiding this comment

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

What does the linewidth in the bar plot have to do with the ffmpeg animation? And if this is changed, why is linewidth used in the remainder of the function, and not ll?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh shoot. Somehow injected a test in there. Thanks for catching that. I'll fix ASAP.

@jklymak jklymak force-pushed the fix-ffmpeg-log-level branch from 5bbacd8 to a408aff Compare March 10, 2018 22:18
@jklymak
Copy link
Member Author

jklymak commented Mar 11, 2018

Mac build is the one that failed (at pip not available)

@dopplershift
Copy link
Contributor

Not going to worry about OSX right now.

@dopplershift dopplershift merged commit 8bc52ad into matplotlib:master Mar 12, 2018
lumberbot-app bot pushed a commit that referenced this pull request Mar 12, 2018
@dopplershift
Backport PR #10739: FIX: ffmpeg logging level
3f212ce
@lumberbot-app lumberbot-app bot mentioned this pull request Mar 12, 2018
Merged
QuLogic added a commit that referenced this pull request Mar 12, 2018
@QuLogic
Merge pull request #10760 from matplotlib/auto-backport-of-pr-10739
a6cdd0f 
Backport PR #10739 on branch v2.2.x
taunsquared added a commit to everymind/SurprisingMinds-Analysis that referenced this pull request Mar 19, 2020
@taunsquared
solved video writing bug
3570e7d 
the logging was killing my memory buffer: matplotlib/matplotlib#10739
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ImportanceOfBeingErnest ImportanceOfBeingErnest ImportanceOfBeingErnest left review comments

@dopplershift dopplershift dopplershift approved these changes

@anntzer anntzer anntzer approved these changes

Assignees
No one assigned
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v2.2.1
Development

Successfully merging this pull request may close these issues.

Animation saving via ffmpeg stops after some 300 frames
4 participants
@jklymak @anntzer @dopplershift @ImportanceOfBeingErnest