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

Fix deep probe last frame #45

Merged
merged 5 commits into from
Nov 21, 2023
Merged

Fix deep probe last frame #45

merged 5 commits into from
Nov 21, 2023

Conversation

Romanelaf
Copy link
Collaborator

nomalab/nomalab#4520

@Romanelaf Romanelaf force-pushed the fix_deep_probe_audio branch from 951ace5 to 5226d97 Compare August 17, 2023 14:41
@Romanelaf Romanelaf force-pushed the fix_deep_probe_last_frame branch 2 times, most recently from 271f507 to a1c331f Compare August 17, 2023 14:42
@Romanelaf Romanelaf requested review from Bramart and Giska August 17, 2023 15:19
@@ -330,6 +330,7 @@ impl DeepProbe {
return Ok(());
}

let mut frame_duration = 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Si tu le definis la, on peut le passer en parametre partout ou on en a besoin, ou bien on le definit tout le temps dans la fonction de la detection.
Je trouve plus propre de l'avoir seulement dans la fonction de detection, mais ca veut dire qu'on le recalcule a chaque fois (pareil pour le frame_rate, ...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tu parles de quelle fonction de détection ici ?

Copy link
Contributor

Choose a reason for hiding this comment

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

blackframes, dualmono, silence, black and silence, je pensais qu'il y en avait plus

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok j'avais pas bien compris ton premier commentaire, c'est vrai que c'est pas très logique de l'avoir mis en paramètre juste pour le black_and_silence, je vais le déplacer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah non en fait c'est qu'on a pas le context dans le black_and_silence, comme c'est une fonction qui utilise des StreamProbeResult déjà existant, c'est un peu dommage de lire à nouveau dans le fichier. Finalement passer les framerate, frame duration etc en paramètre c'est mieux je pense.

Base automatically changed from fix_deep_probe_audio to master November 20, 2023 17:53
@Romanelaf Romanelaf force-pushed the fix_deep_probe_last_frame branch from c561d16 to eaeec0a Compare November 20, 2023 17:55
@Romanelaf Romanelaf requested a review from Bramart November 20, 2023 17:55
@Romanelaf Romanelaf force-pushed the fix_deep_probe_last_frame branch from eaeec0a to 058a60f Compare November 20, 2023 18:19
@Romanelaf Romanelaf requested a review from Bramart November 21, 2023 11:03
@Romanelaf Romanelaf merged commit 04dcb94 into master Nov 21, 2023
11 checks passed
@Romanelaf Romanelaf deleted the fix_deep_probe_last_frame branch November 21, 2023 15:11
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.

2 participants