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

Basic integrity check: reject the DPX stream if 1 of the file is too small #154

Merged
merged 2 commits into from
Sep 23, 2018

Conversation

JeromeMartinez
Copy link
Member

@JeromeMartinez JeromeMartinez commented Sep 23, 2018

One of our beta-testers has a stream with one broken file (file is cut, file transfer error), leading to a crash.
Fixing crash + basic integrity check of the file size (content rejected & file name of the small file is provided).

@JeromeMartinez JeromeMartinez merged commit ec9118f into MediaArea:master Sep 23, 2018
@JeromeMartinez JeromeMartinez deleted the DPX_InvalidSize branch September 23, 2018 13:19
@kieranjol
Copy link
Contributor

This is a great check to make.another common error is a file in the sequence is missing.

@JeromeMartinez
Copy link
Member Author

This is a great check to make.another common error is a file in the sequence is missing.

In that case RAWcooked creates 2 video streams (e.g. files 0-1000, and files 1002-2000, if 1001 is missing), a bit weird for playback but reversibility is OK.
Wondering if I should reject the stream instead, would it make more sense?
We are moving to a DPX conformance checker here ;-).

@retokromer
Copy link
Collaborator

There are real world situation in which missing numbers are correct.

@retokromer
Copy link
Collaborator

E.g. for at least two archives, when we digitise very badly decomposed materials, we have so skip the numbers of the frames we could not digitise.

@JeromeMartinez
Copy link
Member Author

There are real world situation in which missing numbers are correct.

Curious about the reason (edit: you explained when I was writing :) )

Wondering if it is a use case from our sponsors, or a priority for them, to have a single video stream for that kind of directory.
Note that the current command line usage for FFmpeg would not like it, I would like to use a text file for the list of files. More complicated (especially on time stamping), maybe not the the priority and was not in my mind when I estimated development time.

@retokromer
Copy link
Collaborator

Another case that comes in mind is: When a film has two very different stock materials (e.g. reality on reversal and nightmare on negative-positive) which are decomposed differently. The «raw scan» will consist of two successive scans (at least with different settings, and possibly made on two different scanners, and possibly even with a chemical treatment in between the two digitisations). In this case some archives wish to keep the two scans distinct, but by numbering that way the frames, it can be easily imported in the correct order into a timeline for restoration, post-production, grading or whatever.

I would indeed suggest to test the continuity of the numbers. An additional flag could be set to either give an error message (default) or make different streams, when there are missing or «missing» frames. Would this be difficult to implement?

@kieranjol
Copy link
Contributor

kieranjol commented Sep 23, 2018 via email

@JeromeMartinez
Copy link
Member Author

An additional flag could be set to either give an error message (default) or make different streams, when there are missing or «missing» frames. Would this be difficult to implement?

Right now it is a bit complicated because directories traversal and sequences detection are not at the same place (not good first design), but it will be easy when I change this code (I don't like the slowness of the current implementation), I add that to the ToDo-list.
#155: Reto idea
#156: "perfect" handling (single stream with right timestamps)

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.

None yet

3 participants