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

Don't panic if ffmpeg fails to count frames #634

Merged
merged 2 commits into from
Jun 5, 2022

Conversation

Y0ba
Copy link
Contributor

@Y0ba Y0ba commented Jun 5, 2022

Sometimes with broken input num_frames fails to count frames and .unwrap() panics. Unfortunately since it panics in threaded code, this panic does not stop app from running and instead thread just hangs forever and tui shows some weird visual glitches.

@redzic
Copy link
Collaborator

redzic commented Jun 5, 2022

Looks like you need to run cargo fmt, that's why the CI is failing. I think this looks OK for now, but maybe in the future we want to retry the frame count itself instead of restarting the entire chunk.

@Y0ba
Copy link
Contributor Author

Y0ba commented Jun 5, 2022

In my case retrying didn't help at all (it failed constantly). Maybe it will be better to catch all panics in children threads and exit from application?

It's possible to do that with code like this (in main.rs):

use av1an_cli::run;
use std::panic;
use std::process;

fn main() -> anyhow::Result<()> {
  let orig_hook = panic::take_hook();
  panic::set_hook(Box::new(move |panic_info| {
    orig_hook(panic_info);
    process::exit(1);
  }));
  run()
}

@redzic
Copy link
Collaborator

redzic commented Jun 5, 2022

Just out of curiosity, what are the circumstances in which you can reproduce this frame count failure to happen? I ask because maybe there's another solution to that particular cause of this issue.

Your suggestion about catching panics in child threads seems fine to me. I think it'd be better if we do that in a followup PR though.

Copy link
Collaborator

@redzic redzic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Definitely an improvement over the current code.

@mergify mergify bot merged commit b8272ae into master-of-zen:master Jun 5, 2022
@Y0ba
Copy link
Contributor Author

Y0ba commented Jun 5, 2022

Just out of curiosity, what are the circumstances in which you can reproduce this frame count failure to happen?

lsmash failed to open .VOB-file. Either using ffms2 or remuxing into .mkv fixed the error.

@redzic
Copy link
Collaborator

redzic commented Jun 5, 2022

Yeah, in that case retrying wouldn't help at all. Well at least this error doesn't seem to happen in situations where it shouldn't.

@Y0ba Y0ba deleted the fix_unwrap branch June 5, 2022 21:54
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

2 participants