add bitrate option to ffmpeg call

[aestrivex]

I noticed the movies I was creating from pysurfer were sometimes very low quality.

[coveralls]

Coverage decreased (-0.05%) to 72.44% when pulling 69cfaac68129222cafc6dc570b5fe8da21be5a32 on aestrivex:add_movie_bitrate_api into b8b8c1017e85ab3d69259d1495c3db0316aab582 on nipy:master.

[coveralls]

Coverage remained the same at 72.49% when pulling 69cfaac68129222cafc6dc570b5fe8da21be5a32 on aestrivex:add_movie_bitrate_api into b8b8c1017e85ab3d69259d1495c3db0316aab582 on nipy:master.

[christianbrodbeck]

Implementation looks good, I wonder if it's a good idea to set the default as constant, i.e. to have the same bitrate whether you are making a high resolution movie with 6 brains or a small one with a single brain. Maybe the default should still be None and that should call ffmpeg without an argument?

[aestrivex]

I picked the default value to be approximately the same value (i.e., movies that gave me similar performance) to setting no argument. It turns out that according to the ffmpeg documentation the default bitrate is 200k and not 750k as I specified, which is still pretty poor quality. So I would prefer to change the default value to that but keep it in the code to be explicit.

On Tue, May 5, 2015 at 9:32 PM, Christian Brodbeck <notifications@github.com

wrote:

Implementation looks good, I wonder if it's a good idea to set the default as constant, i.e. to have the same bitrate whether you are making a high resolution movie with 6 brains or a small one with a single brain. Maybe the default should still be None and that should call ffmpeg without an argument?

— Reply to this email directly or view it on GitHub https://github.com/nipy/PySurfer/pull/130#issuecomment-99282499.

[christianbrodbeck]

So you're saying 750k looks approximately the same as 200k?

[aestrivex]

For the movies and resolution I made, they are pretty comparable. I cut it up to 2m and it's considerably better. But anyway it seems reasonable to use ffmpeg's default and let the user adjust it.

On Tue, May 5, 2015 at 10:11 PM, Christian Brodbeck < notifications@github.com> wrote:

So you're saying 750k looks approximately the same as 200k?

— Reply to this email directly or view it on GitHub https://github.com/nipy/PySurfer/pull/130#issuecomment-99290910.

[christianbrodbeck]

I think the default could go either way, sometimes you want movies that are higher quality and sometimes you want to save several movies for reference without clogging your hard drive... the increase in bitrate would have to be justified by improved quality though, so if 750k looks the same as 200k maybe it's not an ideal default...

[christianbrodbeck]

What would be useful would be a note in the documentation on what bitrate to use to get good quality

[larsoner]

I have a feeling users will want good movies more often than they'd want crappy ones, and at least by default it would be nice to produce something that looks good. So +1 for using 2M or whatever looks good by default for me.

[christianbrodbeck]

Yeah I agree with a default that looks good

[agramfort]

+1 for merge too.

[aestrivex]

Okay, I did a little bit of testing and increased the default value to 1M which seems to be a consistently good value (ffmpeg's behavior is a little strange). My movies are pretty high resolution though so I think for many people a lower bitrate would be acceptable, but the filesize of the movies I generate with this is not so unreasonable (on the order of megabytes or tens of megabytes if the movie is long) so I don't think setting the default here is a big deal.

[larsoner]

Sounds good, I'll merge once Travis comes back happy

[coveralls]

Coverage remained the same at 72.49% when pulling f206b015bc4d7078d9bd50b37bdcf46459a62c9c on aestrivex:add_movie_bitrate_api into b8b8c1017e85ab3d69259d1495c3db0316aab582 on nipy:master.

[larsoner]

Thanks @aestrivex