Optimize hflip #148

Adib234 · 2021-11-19T17:34:32Z

Summary

[X ] I have read CONTRIBUTING.md to understand how to contribute to this repository :)

<Please summarize what you are trying to achieve, what changes you made, and how they acheive the desired result.>
I am trying to speed up horizontally flipping, and what I did is use VidGear to execute an ffmpeg command with the preset of ultrafast.

With my changes in running HFlip tests: 4.724 seconds (tests would complete around 2.5 seconds)
Without my changes in running HFlip tests: 12.534 seconds (tests would complete around 10.5 seconds)

I've run each test with and without my changes five times on fish shell with this command time for i in (seq 5); python -m unittest augly.tests.video_tests.transforms.ffmpeg_test.TransformsVideoUnitTest.test_HFlip;; end

Unit Tests

If your changes touch the audio module, please run all of the audio tests and paste the output here. Likewise for image, text, & video. If your changes could affect behavior in multiple modules, please run the tests for all potentially affected modules. If you are unsure of which modules might be affected by your changes, please just run all the unit tests.

Audio

python -m unittest discover -s augly/tests/audio_tests/ -p "*"

Image

python -m unittest discover -s augly/tests/image_tests/ -p "*_test.py"
# Or `python -m unittest discover -s augly/tests/image_tests/ -p "*.py"` to run pytorch test too (must install `torchvision` to run)

Text

python -m unittest discover -s augly/tests/text_tests/ -p "*"

Video

python -m unittest discover -s augly/tests/video_tests/ -p "*"

All

python -m unittest discover -s augly/tests/ -p "*"

Other testing

If applicable, test your changes and paste the output here. For example, if your changes affect the requirements/installation, then test installing augly in a fresh conda env, then make sure you are able to import augly & run the unit test

Summary: Sometimes `extract_audio_to_file()` only extracts the beginning of an audio clip, not the full one. This issue was identified in Adib (MLH intern)'s [PR](facebookresearch#133) adding `augment_audio` because the audio metadata had a src & dst duration way shorter than that of the video. - Changed the code in `extract_audio_to_file()` to be more like [this example](https://www.kaggle.com/josecarmona/ffmpeg-python-example-to-extract-audio-from-mp4) I found, which seems to work better for wav files - If the audio file is aac format, run the old version of the function (because the new version isn't compatible with aac) Differential Revision: D31378945 fbshipit-source-id: 38543c0a774e24800d7dc00032d3c9e48ffaa0c8

facebook-github-bot · 2021-12-06T09:17:27Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-12-06T09:23:01Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

zpapakipos · 2021-12-06T09:54:40Z

augly/video/augmenters/ffmpeg/base_augmenter.py

+
+    @abstractmethod
+    def get_command(
+        self, video_path: str, output_path: Optional[str] = None


Suggested change

self, video_path: str, output_path: Optional[str] = None

self, video_path: str, output_path: str

zpapakipos · 2021-12-06T09:55:00Z

augly/video/augmenters/ffmpeg/base_augmenter.py

+        @param video_path: the path to the video to be augmented
+
+        @param output_path: the path in which the resulting video will be stored.
+            If not passed in, the original video file will be overwritten


Suggested change

If not passed in, the original video file will be overwritten

zpapakipos · 2021-12-06T09:55:18Z

augly/video/augmenters/ffmpeg/hflip.py

-    ) -> Tuple[FilterableStream, Dict]:
+class VideoAugmenterByHFlip(BaseVidgearFFMPEGAugmenter):
+    def get_command(
+        self, video_path: str, output_path: Optional[str] = None, **kwargs


Suggested change

self, video_path: str, output_path: Optional[str] = None, **kwargs

self, video_path: str, output_path: str = None, **kwargs

zpapakipos · 2021-12-06T09:55:29Z

augly/video/augmenters/ffmpeg/hflip.py

-        @returns: a tuple containing the FFMPEG object with the augmentation
-            applied and a dictionary with any output arguments as necessary
+        @param output_path: the path in which the resulting video will be stored.
+            If not passed in, the original video file will be overwritten


Suggested change

If not passed in, the original video file will be overwritten

…path and output path

…o optimize-hflip * 'optimize-hflip' of https://github.com/Adib234/AugLy: Update requirements.txt

facebook-github-bot · 2021-12-06T15:38:54Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-12-06T15:40:47Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-12-06T15:48:30Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

augly/video/augmenters/ffmpeg/hflip.py

facebook-github-bot · 2021-12-06T17:26:22Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-12-06T17:27:09Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

augly/video/augmenters/ffmpeg/base_augmenter.py

requirements.txt

augly/video/augmenters/ffmpeg/base_augmenter.py

zpapakipos · 2021-12-07T09:55:35Z

augly/video/augmenters/ffmpeg/base_augmenter.py

+    @abstractmethod
+    def get_command(self, video_path: str, output_path: str) -> List[str]:
+        """
+        Constructs the FFMPEG so that VidGear can run it


Suggested change

Constructs the FFMPEG so that VidGear can run it

Constructs the FFMPEG command for VidGear

augly/video/augmenters/ffmpeg/base_augmenter.py

augly/video/augmenters/ffmpeg/hflip.py

augly/video/functional.py

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

facebook-github-bot · 2021-12-07T14:46:32Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-12-07T15:33:31Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

jbitton · 2021-12-07T18:06:15Z

augly/video/augmenters/ffmpeg/hflip.py

+            the augmentation
+            in a command line


could you remove this extraneous line? :D

Suggested change

the augmentation

in a command line

the augmentation

facebook-github-bot · 2021-12-07T19:07:06Z

@Adib234 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-12-08T08:30:09Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: - [X ] I have read CONTRIBUTING.md to understand how to contribute to this repository :) <Please summarize what you are trying to achieve, what changes you made, and how they acheive the desired result.> I am trying to speed up horizontally flipping, and what I did is use VidGear to execute an ffmpeg command with the preset of ultrafast. With my changes in running HFlip tests: 4.724 seconds (tests would complete around 2.5 seconds) Without my changes in running HFlip tests: 12.534 seconds (tests would complete around 10.5 seconds) I've run each test with and without my changes five times on fish shell with this command `time for i in (seq 5); python -m unittest augly.tests.video_tests.transforms.ffmpeg_test.TransformsVideoUnitTest.test_HFlip;; end` ## Unit Tests If your changes touch the `audio` module, please run all of the `audio` tests and paste the output here. Likewise for `image`, `text`, & `video`. If your changes could affect behavior in multiple modules, please run the tests for all potentially affected modules. If you are unsure of which modules might be affected by your changes, please just run all the unit tests. ### Audio ```bash python -m unittest discover -s augly/tests/audio_tests/ -p "*" ``` ### Image ```bash python -m unittest discover -s augly/tests/image_tests/ -p "*_test.py" # Or `python -m unittest discover -s augly/tests/image_tests/ -p "*.py"` to run pytorch test too (must install `torchvision` to run) ``` ### Text ```bash python -m unittest discover -s augly/tests/text_tests/ -p "*" ``` ### Video ```bash python -m unittest discover -s augly/tests/video_tests/ -p "*" ``` ### All ```bash python -m unittest discover -s augly/tests/ -p "*" ``` ## Other testing If applicable, test your changes and paste the output here. For example, if your changes affect the requirements/installation, then test installing augly in a fresh conda env, then make sure you are able to import augly & run the unit test Pull Request resolved: facebookresearch#148 Reviewed By: jbitton Differential Revision: D32591542 Pulled By: zpapakipos fbshipit-source-id: 83116e0ffe83c1682c6255ac2f65b58716f088f7

…oise-vflip * upstream/main: Optimize hflip (facebookresearch#148) increasing highpass efficiency (facebookresearch#149)

…34/AugLy into optimize-loop-add_noise-vflip * 'optimize-merge-video-audio' of https://github.com/Adib234/AugLy: (31 commits) Optimize hflip (facebookresearch#148) increasing highpass efficiency (facebookresearch#149) added newline to match upstream main manually changed base_unit_test.py to be in sync with upstream/main Remove ci_testing workflow that I was testing Remove making new conda env Add conda init Fix spaces Use conda Fix python step Merge python & reqs steps Add CI testing Github action added vidgear with version number format with black replaced video_path with output_path removed using the same file path for a ffmpeg command using tempfile instead of actually creating a file added vidgear as a dependency used newer format removed uncommented code ...

…34/AugLy into optimize-merge-video-audio * 'optimize-merge-video-audio' of https://github.com/Adib234/AugLy: (31 commits) Optimize hflip (facebookresearch#148) increasing highpass efficiency (facebookresearch#149) added newline to match upstream main manually changed base_unit_test.py to be in sync with upstream/main Remove ci_testing workflow that I was testing Remove making new conda env Add conda init Fix spaces Use conda Fix python step Merge python & reqs steps Add CI testing Github action added vidgear with version number format with black replaced video_path with output_path removed using the same file path for a ffmpeg command using tempfile instead of actually creating a file added vidgear as a dependency used newer format removed uncommented code ...

Adib234 and others added 30 commits September 29, 2021 18:35

added function-based augment-audio

4dc782b

format

487d72a

running test results in error

9763342

type error and runtime error while testing

e5fd64e

ffprobe error in test

526dbe6

positional argument error

5a0d765

changed audio_aug to aug_function

ca234b7

augment_audio test passes

9014ba4

fixed formatting issues and logged metadata

73ec89a

added function-based augment-audio

2614838

format

115580b

running test results in error

a4671b3

type error and runtime error while testing

b0b66da

ffprobe error in test

1ed1600

positional argument error

6b6c5cd

changed audio_aug to aug_function

c1acd83

augment_audio test passes

836155a

fixed formatting issues and logged metadata

a7bac3f

positional argument error in test_AugmentAudio

601afe7

fixed merge conflicts

4b5e93f

time differences fail tests

21ad412

added correct expected_metadata

4434d44

changed None to null and docstrings

d755063

removed print statements

0ba5f89

fixed formatting nits and now support for class based augmentations

4a74124

Add CI testing Github action

bc0aad0

Merge python & reqs steps

ff81999

Fix python step

595e5af

Use conda

aa78bdc

Update requirements.txt

63d760b

zpapakipos reviewed Dec 6, 2021

View reviewed changes

zpapakipos requested changes Dec 6, 2021

View reviewed changes

Adib234 added 2 commits December 6, 2021 10:37

fixed documentation and avoided duplication of checking for same vid …

a72b32e

…path and output path

Merge branch 'optimize-hflip' of https://github.com/Adib234/AugLy int…

da42e5d

…o optimize-hflip * 'optimize-hflip' of https://github.com/Adib234/AugLy: Update requirements.txt

format base_augmenter.py with black

2bd326b

Merge branch 'facebookresearch:main' into optimize-hflip

b95b0ea

zpapakipos reviewed Dec 6, 2021

View reviewed changes

augly/video/augmenters/ffmpeg/hflip.py Outdated Show resolved Hide resolved

Update augly/video/augmenters/ffmpeg/hflip.py

3fdbd4d

zpapakipos requested changes Dec 7, 2021

View reviewed changes

Apply suggestions from code review

113b7d4

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

jbitton reviewed Dec 7, 2021

View reviewed changes

remove extraneous line

a8a6c2a

facebook-github-bot closed this in 35a6a5d Dec 8, 2021

Adib234 added a commit to Adib234/AugLy that referenced this pull request Dec 8, 2021

Merge remote-tracking branch 'upstream/main' into optimize-loop-add_n…

6e7daa0

…oise-vflip * upstream/main: Optimize hflip (facebookresearch#148) increasing highpass efficiency (facebookresearch#149)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize hflip #148

Optimize hflip #148

Adib234 commented Nov 19, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

zpapakipos Dec 6, 2021

zpapakipos Dec 6, 2021

zpapakipos Dec 6, 2021

zpapakipos Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

zpapakipos Dec 7, 2021

facebook-github-bot commented Dec 7, 2021

facebook-github-bot commented Dec 7, 2021

jbitton Dec 7, 2021

facebook-github-bot commented Dec 7, 2021

facebook-github-bot commented Dec 8, 2021

	self, video_path: str, output_path: Optional[str] = None
	self, video_path: str, output_path: str

	self, video_path: str, output_path: Optional[str] = None, **kwargs
	self, video_path: str, output_path: str = None, **kwargs

	Constructs the FFMPEG so that VidGear can run it
	Constructs the FFMPEG command for VidGear

Optimize hflip #148

Optimize hflip #148

Conversation

Adib234 commented Nov 19, 2021

Summary

Unit Tests

Audio

Image

Text

Video

All

Other testing

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

zpapakipos Dec 6, 2021

Choose a reason for hiding this comment

zpapakipos Dec 6, 2021

Choose a reason for hiding this comment

zpapakipos Dec 6, 2021

Choose a reason for hiding this comment

zpapakipos Dec 6, 2021

Choose a reason for hiding this comment

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

facebook-github-bot commented Dec 6, 2021

zpapakipos Dec 7, 2021

Choose a reason for hiding this comment

facebook-github-bot commented Dec 7, 2021

facebook-github-bot commented Dec 7, 2021

jbitton Dec 7, 2021

Choose a reason for hiding this comment

facebook-github-bot commented Dec 7, 2021

facebook-github-bot commented Dec 8, 2021