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

Boxing completion detection needs improvement? #644

Closed
wn-dev opened this issue Nov 30, 2021 · 8 comments
Closed

Boxing completion detection needs improvement? #644

wn-dev opened this issue Nov 30, 2021 · 8 comments

Comments

@wn-dev
Copy link

wn-dev commented Nov 30, 2021

Recently encountered an issue where the boxing process (MP4Box) failed on a .h264 file, causing the boxing queue to eventually become full and all subsequent videos to not get boxed, resulting in "Busy" being shown on the preview page for all those videos. It would be great if the mechanism for detecting failures in the boxing process could be improved.

It looks like the current detection mechanism periodically checks whether the .h264 file being boxed is still present, and if so, assumes that the boxing process is still running, even if it failed (i.e. MP4Box exited with an error). Maybe the detection mechanism could be enhanced to also check whether the boxing process is still running?

Also, if errors from the boxing process could be logged by default, that would be quite helpful in troubleshooting. I ended up having to manually change the MP4Box_cmd in /etc/raspimjpeg to have MP4Box log to a file, to determine that it was MP4Box failing with an error when trying to box a particular .h264 file.

@roberttidey
Copy link
Collaborator

OK. I can look into that but have some other Bullseye related tasks to complete first.

One issue is that the MP4Box process is launched asynchronously so there is no direct feedback on failure. One possible method would to relabel the file on failure making it automatically move on.

Can you share the modified command you used as an adaptation of that might do the trick?

@wn-dev
Copy link
Author

wn-dev commented Dec 6, 2021

The following is my replacement for the default MP4Box_cmd in the /etc/raspimjpeg file. It's working for me so far, though it's only encountered one corrupted .h264 file in actual use during the time I've had it in place.

MP4Box_cmd (set -e;FPS=%i;TNAME='%s';FNAME='%s';TNAME='%s';LOGS="$TNAME.log";rm -f "$FNAME";if MP4Box -fps $FPS -add "$TNAME" "$FNAME" > "$LOGS" 2>&1;then touch -r "$TNAME" "$FNAME"; rm "$TNAME" "$LOGS";else mv "$TNAME" "$TNAME.bad";fi;) &

A quick overview of the above script:

  1. Capture the arguments passed to it from the raspimjpeg program.
  2. Remove the target .mp4 file in case it exists, such as from a prior execution of the script that gets interrupted before the source .h264 file was deleted.
  3. Run the MP4Box program, redirecting output messages to a log file.
  4. If MP4Box succeeds:
    • The file times of the .h264 file are copied to the .mp4 file, to avoid erroneously large duration values being shown by preview.php for videos that were boxed much later in time than when the videos were originally recorded.
    • The .h264 and log files are deleted.
  5. If MP4Box fails, the .h264 file is renamed by appending .bad to its filename.

@roberttidey
Copy link
Collaborator

roberttidey commented Dec 7, 2021

Thanks. I would like to replace the install version with this.

One question. TNAME seems to be set twice? Seems OK if I remove the second one.

@wn-dev
Copy link
Author

wn-dev commented Dec 7, 2021

I figured it was safer to set it twice, since the original MP4Box_cmd string had four placeholders, two of which were for the filename of the temporary input file:
MP4Box_cmd (set -e;MP4Box -fps %i -add %s %s > /dev/null 2>&1;rm "%s";) &

And it looked like the code that uses the MP4Box_cmd string to produce the final command string passed the value of filename_temp twice:
https://github.com/roberttidey/userland/blob/08cceb32176aecec592929b450c126f7d3e97560/host_applications/linux/apps/raspicam/RaspiMUtils.c#L452
asprintf(&cmd_temp, cfg_stru[c_MP4Box_cmd], cfg_val[c_MP4Box_fps], filename_temp, box_files[box_tail], filename_temp);

@roberttidey
Copy link
Collaborator

OK That makes sense.

The original code for MP4Box_Cmd did pass a different value for the 4th parameter but when it was updated it became the same as the 2nd. It was duplicated to keep compatibility.

I have updated the value in raspimjpeg to use both copies of the parameter like yours so that all parameters are used.

@wn-dev
Copy link
Author

wn-dev commented Dec 8, 2021

Does the code in the scheduler that automatically purges old files need to be updated to also delete files with the .bad and .log file extensions? Or does that code already delete old files regardless of their file extensions?

@roberttidey
Copy link
Collaborator

The routine that deletes files in config.php needed to be updated to deal with the extra extensions. That then deals with the cases where it is manually deleted or by the schedule purge.

Latest version has the changes needed which should work OK, but I never get this type of error myself. I did a simulation which worked.

@wn-dev
Copy link
Author

wn-dev commented Dec 13, 2021

Thank you for making the changes.

@wn-dev wn-dev closed this as completed Dec 13, 2021
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

No branches or pull requests

2 participants