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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict the video channels in exports #754

Merged
merged 2 commits into from Nov 26, 2016

Conversation

Projects
None yet
3 participants
@grigorisg9gr
Member

grigorisg9gr commented Nov 22, 2016

The existing system works fine for 1 or 3 channels lists of images, however it outputs noise when provided with a different setting, e.g. 4 channel images.
For instance, if matplotlib is used, then the images exported have alpha channel as well, resulting in a video with non-sense content.

Code snippet to reproduce an instance:

import menpo.io as mio
import matplotlib.pyplot as plt

im = mio.import_builtin_asset.lenna_png()

plt.figure()
plt.subplot(131)
renderer = im.view()

plt.subplot(132)
renderer = im.view(new_figure=False)
plt.subplot(133)
renderer = im.view(new_figure=False)
renderer.save_figure('/tmp/lenna.png', overwrite=True)
im = mio.import_image('/tmp/lenna.png', normalize=False)
print(im.n_channels)
mio.export_video([im, im, im, im, im], '/tmp/lenna.mp4', overwrite=True, fps=2)

Alternative solutions could be provided, e.g. if we export the 3 first channels it should work, but this is the explicit solution, so that the user has the choice.

@jabooth

This comment has been minimized.

Member

jabooth commented Nov 22, 2016

This looks good to me @grigorisg9gr, nice catch!
Can we think of any use case where n_channels not in {1, 3} makes sense for video?

@grigorisg9gr

This comment has been minimized.

Member

grigorisg9gr commented Nov 22, 2016

Several, however in the way that the channels are hardcoded now, i.e. https://github.com/menpo/menpo/blob/master/menpo/io/output/video.py#L66, this line has to be changed for non {1,3} channels. Obviously the 4 channels case might be very useful if the alpha channel is required, e.g. for opacity, but few things need to be adapted in this implementation first.

im = images[0]
frame_shape = im.shape
if im.n_channels != 3 and im.n_channels != 1:
m = ('Currently only images of 1 or 3 channels are expected, '

This comment has been minimized.

@patricksnape

patricksnape Nov 25, 2016

Contributor

These brackets are unnecessary.

This comment has been minimized.

@grigorisg9gr

grigorisg9gr Nov 26, 2016

Member

Are you sure? Probably you mean that you prefer the '' sign in the end instead of the (), is that right?

This comment has been minimized.

@patricksnape

patricksnape Nov 26, 2016

Contributor

My bad - never mind!

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Nov 25, 2016

Good catch - but does MP4 even support an alpha channel?

@patricksnape patricksnape merged commit 80a6ce4 into menpo:master Nov 26, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
macOS MenpoBot Jenkins build passed
Details

@patricksnape patricksnape deleted the grigorisg9gr:video_channels branch Nov 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment