fix: key frame extraction (DEV-1513) #2300
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seakayone
requested review from
irinaschubert and
subotic
and removed request for
irinaschubert
seakayone
requested review from
irinaschubert,
BalduinLandolt,
jnussbaum and
mpro7
seakayone
force-pushed
the
fix/key-frame-extraction-DEV-1513
branch
from
4676c77
to
547a3fa
Compare
seakayone
changed the title
|
Before merging I should probably remove the huge .mp4 from the test_data folder as @jnussbaum pointed out to me. Keeping them for now for a convenience for our reviewers. |
|
in the PR title, "DEV" should be all-caps and in parentheses :) |
BalduinLandolt
approved these changes
Nov 23, 2022
Codecov ReportBase: 86.95% // Head: 86.95% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #2300 +/- ##
=======================================
Coverage 86.95% 86.95%
=======================================
Files 242 242
Lines 28102 28102
=======================================
Hits 24435 24435
Misses 3667 3667 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
seakayone
changed the title
seakayone
changed the title
irinaschubert
approved these changes
Nov 23, 2022
I will remove the "test_data/*.mp4" files before merging. I did not add any tests because:
|
seakayone
force-pushed
the
fix/key-frame-extraction-DEV-1513
branch
2 times, most recently
from
d5ad6be
to
210d5cf
Compare
Fixes bugs which prevent the key frame extraction and matrix file creation of the two files: https://github.com/dasch-swiss/4124-xmlperformance-testdata/blob/main/bitstreams/0.1.mp4 https://github.com/dasch-swiss/4124-xmlperformance-testdata/blob/main/bitstreams/100.mp4 Both files suffer from two different bugs: Fixes processing of very short moving images (0.1.mp4) The file is too short for any frame to be extracted with the previous implementation which assumed at least five keyframes to be extracted. Change the script to start extracting keyframes at time unit 0, i.e. the beginning of the file and use the first extract frame as the start of the matrix instead of a later possibly non existing one. Fix rounding error in calculation of number of matrix files to created (100.mp4) For some reason the calculation of the number of matrix files was rounded from 0.8... to 2 where the next bigger integer should be 1 fixes issue DEV-1513 Additional changes: * Add support for debugging output to export-moving-image-frames.sh Introduce $DEBUG for the export script as a way to turn on debug out * Mount scripts folder into sipi for local dev purposed * Declare function with function keyword * Formatting - use spaces instead of tabs akin to scala source code * Fix mimetype check being skipped/failing Failed with + file -b --mime-type ./1.mp4 -ne video/mp4 Usage: file [-bcCdEhikLlNnprsSvzZ0] [--apple] [--extension] [--mime-encoding] [--mime-type] [-e <testname>] [-F <separator>] [-f <namefile>] [-m <magicfiles>] [-P <parameter=value>] <file> ... file -C [-m <magicfiles>] file [--help] * Use double quotes in order to prevent globbing and word splitting * Remove unused code: function collect, unused num variable,unused framestart variable * Fix warning for read without -r https://github.com/koalaman/shellcheck/wiki/SC2162 * Simplify calculation and rename number_of_frame_files
seakayone
force-pushed
the
fix/key-frame-extraction-DEV-1513
branch
from
210d5cf
to
1cfee67
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number: DEV-1513
Pull Request Checklist
Basic Requirements
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Does this PR change client-test-data?
Other information
This PR contains fixes which prevent the key frame extraction and matrix file creation of the two files:
https://github.com/dasch-swiss/4124-xmlperformance-testdata/blob/main/bitstreams/0.1.mp4
https://github.com/dasch-swiss/4124-xmlperformance-testdata/blob/main/bitstreams/100.mp4
Both files suffer from two different bugs:
Fixes processing of very short moving images (0.1.mp4)
The file is too short for any frame to be extracted with the previous implementation which assumed at least five keyframes to be extracted. Change the script to start extracting keyframes at time unit 0, i.e. the beginning of the file and use the first extract frame as the start of the matrix instead of a later possibly non existing one.
Fix rounding error in calculation of number of matrix files to created (100.mp4)
For some reason the calculation of the number of matrix files was rounded from 0.8... to 2 where the next bigger integer should be 1
fixes issue DEV-1513