Skip to content
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

Danielsf/dev/downsampled/video #415

Merged
merged 42 commits into from
Jan 25, 2022
Merged

Conversation

danielsf
Copy link
Contributor

@danielsf danielsf commented Jan 24, 2022

This PR adds a utility to generate downsampled video files (.avi, .mp4, etc.) for those who do not want to use the .webm files currently output by the motion correction pipeline. There is a module (single_video) for generating a single downsampled video and a module (side_by_side_video) for showing two videos side by side (e.g. pre- and post-motion correction). Examples of the side by side video module's output can be found here

/allen/aibs/informatics/danielsf/downsampling_example

This work is not associated with any specific ticket (unless maybe #412), but having this tool will be necessary to enable the science team to evaluate our prospective motion correction configurations.

@danielsf danielsf force-pushed the danielsf/dev/downsampled/video branch from b56f9d4 to 5427308 Compare January 24, 2022 17:42
@danielsf danielsf force-pushed the danielsf/dev/downsampled/video branch from 5d2157a to b9e2abb Compare January 24, 2022 18:17
@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #415 (3602fde) into main (272c74a) will decrease coverage by 0.14%.
The diff coverage is 89.21%.

❗ Current head 3602fde differs from pull request most recent head 8c76d54. Consider uploading reports for the commit 8c76d54 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #415      +/-   ##
==========================================
- Coverage   92.47%   92.32%   -0.15%     
==========================================
  Files          84       88       +4     
  Lines        5182     5423     +241     
==========================================
+ Hits         4792     5007     +215     
- Misses        390      416      +26     
Flag Coverage Δ
event_detection_tests 30.42% <24.89%> (-0.26%) ⬇️
general_tests 90.09% <89.21%> (-0.05%) ⬇️
not_container_tests 90.15% <89.21%> (-0.05%) ⬇️
suite2p_tests 34.27% <24.89%> (-0.44%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ophys_etl/modules/downsample_video/schemas.py 72.34% <72.34%> (ø)
...ophys_etl/modules/downsample_video/single_video.py 81.25% <81.25%> (ø)
...etl/modules/downsample_video/side_by_side_video.py 82.35% <82.35%> (ø)
src/ophys_etl/modules/downsample_video/utils.py 95.65% <95.65%> (ø)

@danielsf danielsf force-pushed the danielsf/dev/downsampled/video branch from 325c5fb to d787f19 Compare January 25, 2022 05:01
@danielsf danielsf marked this pull request as ready for review January 25, 2022 05:07
src/ophys_etl/modules/downsample_video/utils.py Outdated Show resolved Hide resolved
required=False,
default=8,
allow_none=False,
description=("Factor by which to speed up output movie"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the same description here as in the code, which is better

        Factor by which to speed up the movie *after downsampling* when writing
        to video (in case you want a smaller file that can be played back
        faster)

src/ophys_etl/modules/downsample_video/utils.py Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
# empty
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this package should just be called video, as it's possible to apply no downsampling but still create a video

src/ophys_etl/modules/downsample_video/schemas.py Outdated Show resolved Hide resolved
import pathlib


class SideBySideDownsamplerSchema(DownsampleBaseSchema):
Copy link
Contributor

Choose a reason for hiding this comment

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

SideBySideVideoSchema

src/ophys_etl/modules/downsample_video/utils.py Outdated Show resolved Hide resolved
src/ophys_etl/modules/downsample_video/utils.py Outdated Show resolved Hide resolved
@danielsf
Copy link
Contributor Author

The python 3.7 build keeps timing out on CircleCI on tests that this PR does not touch. CircleCI reports itself as in a degraded state. I'm going to merge now on the assumption that that is our problem. If the problem does not resolve itself tomorrow, we can look into it.

@danielsf danielsf merged commit 5159c07 into main Jan 25, 2022
@danielsf danielsf deleted the danielsf/dev/downsampled/video branch February 3, 2022 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants