pick highest fps when concatenating #416

BrianLee608 · 2017-02-15T21:40:33Z

These changes resolve issue 407

tburrows13

LGTM

Zulko · 2017-02-16T00:02:30Z

This forgets the case where all the fps may be None (this happens if you are concatenating a series of still pictures), at which case the concatenation's fps should be None. Here you'll get a bug with the max. Can we add something like this ?

if all([clip.fps is None for clip in clips]):
    self.fps=None

BrianLee608 · 2017-02-16T00:56:20Z

I think something went weird with my revised PR.

But I just changed the logic to:

fpss = [c.fps for c in clips if hasattr(c,'fps')]
    if len(fpss) == 0:
        result.fps = None
    else:
        result.fps = max(fpss)

It avoids looping through the clips twice.

removed resolve markers such as HEAD, etc so that the file will compile correctly. Remove some double blank lines, etc

ghost · 2017-02-27T02:02:28Z

can we commit this PR? It passes the 4 or 5 tests, I have created so far. I also have a test for this issue that needs to be uncommented once this PR is committed. I'll commit it if no one objects by Wednesday March 1st.

Zulko · 2017-02-27T08:13:00Z

Sure go ahead. If it ever go causes trouble in some scenarios it will be easy to revert/refine later.

Zulko · 2017-02-27T08:17:58Z

Wait actually my remark above hasn't been fully addressed. The line should read like this to take into account that some clips have a fps equal to None:

fpss = [c.fps for c in clips if hasattr(c,'fps') and c.fps is not None]

Not relevant

* Fixed copy-paste typo Changed documentation for the color parameter to distinguish from bg_color * Fixed missing list (using python 3) * fixed module hierarchy for Trajectory * fixed addy * small recipe * on_color function docstring has wrong parameter The docstring for the on_color method has a parameter of bg_color but the function uses color as the parameter. * Update README.rst Changed Code Block to use Python Syntax Highlighting * fix deprecation message currently, the docstring is nonsense. ``` The function ``concatenate_videoclips`` is deprecated and is kept temporarily for backwards compatibility. Please use the new name, ``concatenate_videoclips``, instead. ``` * ImageSequenceClip: Check for fps and durations rather than fps and duration * Add a test case. * Add another test * ensures int arguments to np.reshape; closes Zulko#383 * fix issue Zulko#401 * fix issue Zulko#335 * Update maintainer section in README * make concatenate_videoclips Python 3 compatible.. fix issue Zulko#313 * Increment release version * CompositeVideoClip doesn't accept an argument of transparent * move PY3 variable to compat.py * fix movie => moviepy typo * fix issue Zulko#341 * Fixed typo Zulko#375 Zulko#375 fixed * fix issue Zulko#357, which makes real problem more obvious (media file does not exist * Revert "small recipe (mirroring a video)" * Fixed indentation * .gitignore ignore Mac-specific files, Jetbrains settings dir * README.rst make mention of Gitter, add PyPI and Gitter badge * README.rst wording, formatting * README.rst structure link targets, add co-maintainers (with @username) * README.rst move maintainers, contributing sections; change docs x-reference name * README.rst fix grammar * fix issue 145. raise Exception when concatenate method != chain or compose * make PEP8 compatible * fix PR Zulko#413 . (issue Zulko#357) * create test for issue Zulko#145 * add tests/media to .gitignore * fix Issue Zulko#385 , no DirectoryClip class (Zulko#434) * fix issue Zulko#385 , no DirectoryClip class * replace DirectoryClip with ImageSequenceClip * fix issue 417, unicode has no attribute 'shape' error. * add test for issue 417 * add test for issue 417 * Fixed resize documentation issue Zulko#319 (Zulko#346) * Handle bytes when listing fonts in VideoClip.py (Zulko#306) Handle bytes when listing fonts in VideoClip.py * add test for PR306 * add test for PR306 (Zulko#440) * create test file for pull requests (Zulko#433) * Test issue 407 (video has a valid fps after concatenate function) (Zulko#443) * Move PR test to test_PR.py file (Zulko#444) * move PR test from test_issues.py to test_PR.py * add code to download python_logo.png * remove duplicate test_issue_417 function * add testing with travis-ci (Zulko#447) added testing via travis-ci * put DEVNULL into compat.py (Zulko#432) * add travis-ci badge to readme file * pick highest fps when concatenating (Zulko#416) * readers.py cast chunksize from float to int * choose highest fps of clips when concatenating * pick highest fps when concatenating * pick highest fps when concatenating * fps either max or none * remove resolve markers removed resolve markers such as HEAD, etc so that the file will compile correctly. Remove some double blank lines, etc * update concatenate.py; add c.fps is not None * add test for issue 416 * fix test_issue_416

* Changed the `col` paramater of `ColorClip` to `color` and added a DeprecationWarning if the users tries to use `col` * Update VideoFileClip.py * Added warnings * Update (#3) * Fixed copy-paste typo Changed documentation for the color parameter to distinguish from bg_color * Fixed missing list (using python 3) * fixed module hierarchy for Trajectory * fixed addy * small recipe * on_color function docstring has wrong parameter The docstring for the on_color method has a parameter of bg_color but the function uses color as the parameter. * Update README.rst Changed Code Block to use Python Syntax Highlighting * fix deprecation message currently, the docstring is nonsense. ``` The function ``concatenate_videoclips`` is deprecated and is kept temporarily for backwards compatibility. Please use the new name, ``concatenate_videoclips``, instead. ``` * ImageSequenceClip: Check for fps and durations rather than fps and duration * Add a test case. * Add another test * ensures int arguments to np.reshape; closes #383 * fix issue #401 * fix issue #335 * Update maintainer section in README * make concatenate_videoclips Python 3 compatible.. fix issue #313 * Increment release version * CompositeVideoClip doesn't accept an argument of transparent * move PY3 variable to compat.py * fix movie => moviepy typo * fix issue #341 * Fixed typo #375 #375 fixed * fix issue #357, which makes real problem more obvious (media file does not exist * Revert "small recipe (mirroring a video)" * Fixed indentation * .gitignore ignore Mac-specific files, Jetbrains settings dir * README.rst make mention of Gitter, add PyPI and Gitter badge * README.rst wording, formatting * README.rst structure link targets, add co-maintainers (with @username) * README.rst move maintainers, contributing sections; change docs x-reference name * README.rst fix grammar * fix issue 145. raise Exception when concatenate method != chain or compose * make PEP8 compatible * fix PR #413 . (issue #357) * create test for issue #145 * add tests/media to .gitignore * fix Issue #385 , no DirectoryClip class (#434) * fix issue #385 , no DirectoryClip class * replace DirectoryClip with ImageSequenceClip * fix issue 417, unicode has no attribute 'shape' error. * add test for issue 417 * add test for issue 417 * Fixed resize documentation issue #319 (#346) * Handle bytes when listing fonts in VideoClip.py (#306) Handle bytes when listing fonts in VideoClip.py * add test for PR306 * add test for PR306 (#440) * create test file for pull requests (#433) * Test issue 407 (video has a valid fps after concatenate function) (#443) * Move PR test to test_PR.py file (#444) * move PR test from test_issues.py to test_PR.py * add code to download python_logo.png * remove duplicate test_issue_417 function * add testing with travis-ci (#447) added testing via travis-ci * put DEVNULL into compat.py (#432) * add travis-ci badge to readme file * pick highest fps when concatenating (#416) * readers.py cast chunksize from float to int * choose highest fps of clips when concatenating * pick highest fps when concatenating * pick highest fps when concatenating * fps either max or none * remove resolve markers removed resolve markers such as HEAD, etc so that the file will compile correctly. Remove some double blank lines, etc * update concatenate.py; add c.fps is not None * add test for issue 416 * fix test_issue_416 * Update Gloin (#4) * Fixed copy-paste typo Changed documentation for the color parameter to distinguish from bg_color * Fixed missing list (using python 3) * fixed module hierarchy for Trajectory * fixed addy * small recipe * on_color function docstring has wrong parameter The docstring for the on_color method has a parameter of bg_color but the function uses color as the parameter. * Update README.rst Changed Code Block to use Python Syntax Highlighting * fix deprecation message currently, the docstring is nonsense. ``` The function ``concatenate_videoclips`` is deprecated and is kept temporarily for backwards compatibility. Please use the new name, ``concatenate_videoclips``, instead. ``` * ImageSequenceClip: Check for fps and durations rather than fps and duration * Add a test case. * Add another test * ensures int arguments to np.reshape; closes #383 * fix issue #401 * fix issue #335 * Update maintainer section in README * make concatenate_videoclips Python 3 compatible.. fix issue #313 * Increment release version * CompositeVideoClip doesn't accept an argument of transparent * move PY3 variable to compat.py * fix movie => moviepy typo * fix issue #341 * Fixed typo #375 #375 fixed * fix issue #357, which makes real problem more obvious (media file does not exist * Revert "small recipe (mirroring a video)" * Fixed indentation * .gitignore ignore Mac-specific files, Jetbrains settings dir * README.rst make mention of Gitter, add PyPI and Gitter badge * README.rst wording, formatting * README.rst structure link targets, add co-maintainers (with @username) * README.rst move maintainers, contributing sections; change docs x-reference name * README.rst fix grammar * fix issue 145. raise Exception when concatenate method != chain or compose * make PEP8 compatible * fix PR #413 . (issue #357) * create test for issue #145 * add tests/media to .gitignore * fix Issue #385 , no DirectoryClip class (#434) * fix issue #385 , no DirectoryClip class * replace DirectoryClip with ImageSequenceClip * fix issue 417, unicode has no attribute 'shape' error. * add test for issue 417 * add test for issue 417 * Fixed resize documentation issue #319 (#346) * Handle bytes when listing fonts in VideoClip.py (#306) Handle bytes when listing fonts in VideoClip.py * add test for PR306 * add test for PR306 (#440) * create test file for pull requests (#433) * Test issue 407 (video has a valid fps after concatenate function) (#443) * Move PR test to test_PR.py file (#444) * move PR test from test_issues.py to test_PR.py * add code to download python_logo.png * remove duplicate test_issue_417 function * add testing with travis-ci (#447) added testing via travis-ci * put DEVNULL into compat.py (#432) * add travis-ci badge to readme file * pick highest fps when concatenating (#416) * readers.py cast chunksize from float to int * choose highest fps of clips when concatenating * pick highest fps when concatenating * pick highest fps when concatenating * fps either max or none * remove resolve markers removed resolve markers such as HEAD, etc so that the file will compile correctly. Remove some double blank lines, etc * update concatenate.py; add c.fps is not None * add test for issue 416 * fix test_issue_416 * Update VideoFileClip.py * Added test * Added return for PEP 8 compliance * Change

BrianLee608 added 6 commits February 14, 2017 19:18

readers.py cast chunksize from float to int

834952d

choose highest fps of clips when concatenating

2f563ea

concat fps

7dfb5d9

up

f2ab568

pick highest fps when concatenating

c736836

pick highest fps when concatenating

9907504

tburrows13 previously approved these changes Feb 15, 2017

View reviewed changes

Zulko self-requested a review February 16, 2017 00:11

BrianLee608 added 2 commits February 15, 2017 19:47

fps either max or none

6a6909c

fps none or max

bc60e0f

mbeacom added the enhancement Positive change that does not change the API, i.e. improved performance, using less memory etc. label Feb 16, 2017

remove resolve markers

9e675a7

removed resolve markers such as HEAD, etc so that the file will compile correctly. Remove some double blank lines, etc

update concatenate.py; add c.fps is not None

b1f92a2

ghost merged commit c0f6925 into Zulko:master Feb 27, 2017

tburrows13 mentioned this pull request Feb 14, 2018

Fixed bugs, neater code, changed docstrings in audiofiles #722

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pick highest fps when concatenating #416

pick highest fps when concatenating #416

BrianLee608 commented Feb 15, 2017

tburrows13 left a comment

Zulko commented Feb 16, 2017

BrianLee608 commented Feb 16, 2017

ghost commented Feb 27, 2017

Zulko commented Feb 27, 2017

Zulko commented Feb 27, 2017

pick highest fps when concatenating #416

pick highest fps when concatenating #416

Conversation

BrianLee608 commented Feb 15, 2017

tburrows13 left a comment

Choose a reason for hiding this comment

Zulko commented Feb 16, 2017

BrianLee608 commented Feb 16, 2017

ghost commented Feb 27, 2017

Zulko commented Feb 27, 2017

Zulko commented Feb 27, 2017