MRG: Add toggle to save_movie in _TimeViewer #7257

GuillaumeFavelier · 2020-01-27T12:24:12Z

This PR adds a toggle to save the scene displayed in _TimeViewer as a GIF using PyVista movie feature. The shortcut for the toggle is o.

TL:DR: for implementation specific reason, I would push more for offline save_movie() instead, similar to PySurfer. I would like to have your opinion on this @larsoner

Note/Known issue

From my experience of this feature, the interface stutters using the default GIF-PIL (Pillow) format for imageio append_data(). This is probably because disk access is performed at each call. I had better success with GIL-FI (FreeImage) format: the interface was still responsive during recording and suffers for lag only while saving to disk (end of recording).

For reference:

It's an item of #7162

codecov · 2020-01-27T13:04:18Z

Codecov Report

Merging #7257 into master will decrease coverage by 0.01%.
The diff coverage is 85.93%.

@@            Coverage Diff             @@
##           master    #7257      +/-   ##
==========================================
- Coverage   90.13%   90.12%   -0.02%     
==========================================
  Files         454      454              
  Lines       81535    81701     +166     
  Branches    13015    13042      +27     
==========================================
+ Hits        73493    73634     +141     
- Misses       5210     5222      +12     
- Partials     2832     2845      +13

larsoner · 2020-01-27T14:48:20Z

TL:DR: for implementation specific reason, I would push more for offline save_movie() instead, similar to PySurfer. I would like to have your opinion on this @larsoner

I think there are two complementary use cases here. One where you want to save a movie the way PySurfer does (save_movie) with clear use cases. The other is more of a "screencast" mode, where you can click and do various things in order to produce a movie (better for data exploration probably). So it would be nice to have both, but we definitely want the former, and the latter is just a "nice to have" since you could in principle actually use a generic screen-casting tool to get this functionality.

I had better success with GIL-FI (FreeImage) format: the interface was still responsive during recording and suffers for lag only while saving to disk (end of recording).

I'd say we could do one or two of:

add to our environment that imageio get installed with FreeImage support
raise an error when doing o if it's not
check for FreeImage but fall back to PIL if not available, probably with a warn about bad performance

I would vote for 1 and 3.

GuillaumeFavelier · 2020-01-31T20:47:34Z

we definitely want the former, and the latter is just a "nice to have"

I'll go for an offline save_movie() function first and go for screencast in a second phase.

I would vote for 1 and 3.

Sounds like a plan.

This reverts commit 51b5403.

GuillaumeFavelier · 2020-03-05T09:56:13Z

The latest changes:

Add an offline save_movie() function to _Brain similar to PySurfer
Set the default time interpolation to nearest for set_time_point()
Update the overview tables of _Brain and set_backend

GuillaumeFavelier · 2020-03-05T14:27:27Z

Now that save_movie is available in _Brain, should we connect it to the _TimeViewer frontend?

I was thinking of two ways to handle that:

Through a FileDialog to get the output file path
Through a shortcut with a default file path (probably cwd)

But save_movie() has a few parameters so none of my options seems to fit:

mne-python/mne/viz/_brain/_brain.py

Lines 1030 to 1068 in 5b3934f

    
               def save_movie(self, fname, time_dilation=4., tmin=None, tmax=None, 
        
                              framerate=24, interpolation='quadratic', codec=None, 
        
                              bitrate=None, **kwargs): 
        
                   """Save a movie (for data with a time axis). 
        
                   The movie is created through the :mod:`imageio` module. The format is 
        
                   determined by the extension, and additional options can be specified 
        
                   through keyword arguments that depend on the format. For available 
        
                   formats and corresponding parameters see the imageio documentation: 
        
                   http://imageio.readthedocs.io/en/latest/formats.html#multiple-images 
        
                   .. Warning:: 
        
                       This method assumes that time is specified in seconds when adding 
        
                       data. If time is specified in milliseconds this will result in 
        
                       movies 1000 times longer than expected. 
        
                   Parameters 
        
                   ---------- 
        
                   fname : str 
        
                       Path at which to save the movie. The extension determines the 
        
                       format (e.g., `'*.mov'`, `'*.gif'`, ...; see the :mod:`imageio` 
        
                       documenttion for available formats). 
        
                   time_dilation : float 
        
                       Factor by which to stretch time (default 4). For example, an epoch 
        
                       from -100 to 600 ms lasts 700 ms. With ``time_dilation=4`` this 
        
                       would result in a 2.8 s long movie. 
        
                   tmin : float 
        
                       First time point to include (default: all data). 
        
                   tmax : float 
        
                       Last time point to include (default: all data). 
        
                   framerate : float 
        
                       Framerate of the movie (frames per second, default 24). 
        
                   interpolation : str 
        
                       Interpolation method (``scipy.interpolate.interp1d`` parameter, 
        
                       one of 'linear' | 'nearest' | 'zero' | 'slinear' | 'quadratic' | 
        
                       'cubic', default 'quadratic'). 
        
                   **kwargs : 
        
                       Specify additional options for :mod:`imageio`. 
        
                   """

agramfort · 2020-03-05T14:34:57Z

you want to have a save_movie "button" in the 3d window? Not sure what you have in mind

larsoner · 2020-03-05T14:41:40Z

I would make it a menu file dialog File->Save movie (ctrl-shift-s)... below a File->Save image (ctrl-s) ...) dialog

GuillaumeFavelier · 2020-03-05T14:42:44Z

I would make it a menu file dialog

The time for me to design the proof of concept and show the screenshot here but @larsoner explained exactly what I meant with 1)

larsoner · 2020-03-05T14:43:04Z

... and then maybe after you pick a path we have another "options" screen where you can set the other parameters.

But I would say this is a (much) lower priority than some of the other stuff, the high priority would be brain.save_movie as a Python command, and later (lower prio) we can make it nicely accessible via a GUI

agramfort · 2020-03-05T14:47:14Z

+1 GUI integration is a second order pb and yes menu buttons sounds reasonable to me

…

GuillaumeFavelier · 2020-03-05T15:29:46Z

The key binding to ctrl-shift-s is not available yet

larsoner · 2020-03-05T15:38:07Z

mne/viz/_brain/_brain.py

+        times = np.arange(n_frames, dtype=float)
+        times /= framerate * time_dilation
+        times += tmin
+        interp_func = interp1d(self._times, np.arange(self._n_times))


Need to pass interpolation here

larsoner · 2020-03-05T15:38:23Z

mne/viz/_brain/_brain.py

+        except (ImportError, ModuleNotFoundError):
+            has_imageio = False
+            warn("The imageio module is not found, "
+                 "save_movie() will return the image sequence instead")


I wouldn't do this. Let's just raise an error, say that it's required for movie saving, and add to requirements.txt/environment.yml/README.rst

agramfort · 2020-03-05T20:35:42Z

it works already reasonably well. Now 2 things:

it prints stuff at each frame and I suspect it's debug stuff
the framerate in the movie does not match the framerate of the play button. So by default the movie are way too fast. So it's not really usable as it is.

agramfort

although I would suggest to add an entry in the latest.inc page and write some sentences
in https://mne.tools/dev/auto_tutorials/source-modeling/plot_visualize_stc.html#sphx-glr-auto-tutorials-source-modeling-plot-visualize-stc-py saying that movies can be produced.

doc/changes/latest.inc

Co-Authored-By: Alexandre Gramfort <alexandre.gramfort@m4x.org>

larsoner · 2020-03-13T22:00:13Z

Pushed a commit to fix https://circleci.com/gh/mne-tools/mne-python/18592 but @GuillaumeFavelier there are merge conflicts

GuillaumeFavelier · 2020-03-13T22:09:36Z

This was expected. I did a moderate refactor on the latest merged pr about time_viewer. I'll take care of it and rename.

agramfort · 2020-03-14T16:21:49Z

@GuillaumeFavelier let me know when CIs are green after fixing the merge conflicts. Should be good to go then

…

larsoner · 2020-03-16T15:37:25Z

Thanks @GuillaumeFavelier

larsoner · 2020-03-16T21:16:57Z

mne/viz/_brain/_brain.py

                if time.shape != (array.shape[-1],):
                    raise ValueError('time has shape %s, but need shape %s '
                                     '(array.shape[-1])' %
                                     (time.shape, (array.shape[-1],)))


@GuillaumeFavelier this merge commit that you made undid the nice refactoring that made use of _time_interp_funcs, we had a single path for interpolation in place and this undid it. It's why CircleCI is headed for failure on master:

https://circleci.com/gh/mne-tools/mne-python/18662

Can you look to see if this can be removed?

My bad for the regression

Wait are you sure though? because I added _interpolate_data afterwards.

I fixed it in: 82b016a

I opened #7462

This reverts commit 7f6120b.

* Revert "MRG: Add toggle to save_movie in _TimeViewer (#7257)" This reverts commit 7f6120b. * Revert _interpolate_data signature

Add toggle to save_movie

51b5403

GuillaumeFavelier added the VIZ label Jan 27, 2020

GuillaumeFavelier self-assigned this Jan 27, 2020

GuillaumeFavelier mentioned this pull request Jan 27, 2020

ENH: New interface for Brain #7162

Closed

86 tasks

GuillaumeFavelier changed the title ~~Add toggle to save_movie in _TimeViewer~~ WIP: Add toggle to save_movie in _TimeViewer Jan 27, 2020

GuillaumeFavelier added 5 commits March 5, 2020 10:36

Revert "Add toggle to save_movie"

9ec26dc

This reverts commit 51b5403.

Merge branch 'master' into time_viewer_movie

291e15d

Add save_movie to _Brain and change default interpolation

3153ced

Add docstring to save_movie

8f18f30

Update overview tables and fix docstring

5b3934f

Bind save_movie to the menu

80e8904

larsoner reviewed Mar 5, 2020

View reviewed changes

GuillaumeFavelier added 4 commits March 5, 2020 16:51

Refactor imageio import

6f26c80

Update requirements

c45f494

Update key binding

2397825

Merge branch 'master' into time_viewer_movie

7532233

GuillaumeFavelier changed the title ~~WIP: Add toggle to save_movie in _TimeViewer~~ MRG: Add toggle to save_movie in _TimeViewer Mar 5, 2020

Use playback_speed as time_dilation parameter

8e6026a

agramfort approved these changes Mar 13, 2020

View reviewed changes

GuillaumeFavelier changed the title ~~Add toggle to save_movie in _TimeViewer~~ WIP: Add toggle to save_movie in _TimeViewer Mar 13, 2020

Update latest.inc and plot_visualize_stc

23c8dd1

agramfort reviewed Mar 13, 2020

View reviewed changes

doc/changes/latest.inc Outdated Show resolved Hide resolved

GuillaumeFavelier and others added 2 commits March 13, 2020 17:13

Update doc/changes/latest.inc

afb52cd

Co-Authored-By: Alexandre Gramfort <alexandre.gramfort@m4x.org>

Merge branch 'master' into time_viewer_movie

9c21461

GuillaumeFavelier changed the title ~~WIP: Add toggle to save_movie in _TimeViewer~~ MRG: Add toggle to save_movie in _TimeViewer Mar 13, 2020

FIX: Fix for singleton

da82289

GuillaumeFavelier changed the title ~~MRG: Add toggle to save_movie in _TimeViewer~~ WIP: Add toggle to save_movie in _TimeViewer Mar 13, 2020

GuillaumeFavelier added 2 commits March 16, 2020 10:53

Merge branch 'master' into time_viewer_movie

6bbc2ab

Merge branch 'master' into time_viewer_movie

ca62a93

GuillaumeFavelier changed the title ~~WIP: Add toggle to save_movie in _TimeViewer~~ MRG: Add toggle to save_movie in _TimeViewer Mar 16, 2020

GuillaumeFavelier mentioned this pull request Mar 16, 2020

WIP: Set time_viewer and show_traces to auto #7380

Merged

larsoner merged commit 7f6120b into mne-tools:master Mar 16, 2020

larsoner reviewed Mar 16, 2020

View reviewed changes

GuillaumeFavelier mentioned this pull request Mar 16, 2020

Use _time_interp_funcs in _interpolate_data #7462

Merged

GuillaumeFavelier added a commit to GuillaumeFavelier/mne-python that referenced this pull request Mar 17, 2020

Revert "MRG: Add toggle to save_movie in _TimeViewer (mne-tools#7257)"

fe70356

This reverts commit 7f6120b.

agramfort pushed a commit that referenced this pull request Mar 17, 2020

TST: Reproduce paintEvent failures (#7294)

95ad0ee

* Revert "MRG: Add toggle to save_movie in _TimeViewer (#7257)" This reverts commit 7f6120b. * Revert _interpolate_data signature

This was referenced Mar 17, 2020

TST: Reproduce paintEvent failures #7294

Merged

MRG, FIX: Fix for singleton and efficiency #7469

Merged

This was referenced Apr 7, 2020

MRG: Brain save movie #7574

Merged

Rework _Brain show #7580

Merged

MRG: Configure _TimeViewer status bar #7588

Merged

WIP: Time viewer tool bar with SVG icons #7589

Merged

GuillaumeFavelier mentioned this pull request Apr 16, 2020

MRG: TimeViewer save movie #7612

Merged

GuillaumeFavelier deleted the time_viewer_movie branch June 11, 2020 09:41

Uh oh!

MRG: Add toggle to save_movie in _TimeViewer #7257

MRG: Add toggle to save_movie in _TimeViewer #7257

Uh oh!

Conversation

GuillaumeFavelier commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

larsoner commented Jan 27, 2020

Uh oh!

GuillaumeFavelier commented Jan 31, 2020

Uh oh!

GuillaumeFavelier commented Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeFavelier commented Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agramfort commented Mar 5, 2020

Uh oh!

larsoner commented Mar 5, 2020

Uh oh!

GuillaumeFavelier commented Mar 5, 2020

Uh oh!

larsoner commented Mar 5, 2020

Uh oh!

agramfort commented Mar 5, 2020 via email

Uh oh!

GuillaumeFavelier commented Mar 5, 2020

Uh oh!

larsoner Mar 5, 2020

Choose a reason for hiding this comment

Uh oh!

larsoner Mar 5, 2020

Choose a reason for hiding this comment

Uh oh!

agramfort commented Mar 5, 2020

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

larsoner commented Mar 13, 2020

Uh oh!

GuillaumeFavelier commented Mar 13, 2020

Uh oh!

agramfort commented Mar 14, 2020 via email

Uh oh!

larsoner commented Mar 16, 2020

Uh oh!

larsoner Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

GuillaumeFavelier Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

GuillaumeFavelier Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

GuillaumeFavelier Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

GuillaumeFavelier Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GuillaumeFavelier commented Jan 27, 2020 •

edited

Loading

codecov bot commented Jan 27, 2020 •

edited

Loading

GuillaumeFavelier commented Mar 5, 2020 •

edited

Loading

GuillaumeFavelier commented Mar 5, 2020 •

edited

Loading