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

refactor item queue #183

Merged
merged 20 commits into from
Jun 15, 2024
Merged

refactor item queue #183

merged 20 commits into from
Jun 15, 2024

Conversation

rlauuzo
Copy link
Collaborator

@rlauuzo rlauuzo commented May 26, 2024

No description provided.

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented May 26, 2024

  • Adds blacklist check if timestamps are invalid  #177 (comment)
  • Removes path restriction from automatic tasks and passes item IDs to the analyzer instead.
  • Rewrote ChapterAnalyzer using LINQ.
  • ChromaAnalyzer tries to create fingerprints even when only a single episode is provided.

@AbandonedCart
Copy link
Collaborator

The path restrictions was a super hacky way about it. Glad to see it go.

@jumoog
Copy link
Owner

jumoog commented May 26, 2024

so we can remove PathRestrictions from the config?

@AbandonedCart
Copy link
Collaborator

It’s still being used right now, but this PR will make it obsolete

@jumoog
Copy link
Owner

jumoog commented May 26, 2024

Yes, that is my point, if it is obsolete we should remove it from the configuration page to avoid confusion.

@AbandonedCart
Copy link
Collaborator

AbandonedCart commented May 26, 2024

It’s not in the configuration page. It’s a silent option. It’s a preference only for the sake of existing across multiple files without needing to be passed.

(It was really hacky)

@jumoog
Copy link
Owner

jumoog commented May 27, 2024

Oh yes, I just checked the code. Sorry for the confusion.

@AbandonedCart
Copy link
Collaborator

AbandonedCart commented May 29, 2024

I guess that confirms a question i've always had in the back of my mind. Does anime even have the black frames?

(I always assumed it was so subtle that I just didn't see it)

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented May 29, 2024

Animes on Netflix often have a standard anime outro followed by black frames displaying credits for all the languages and dubs.

I've also encountered false positives in shows with a black frame announcing the next episode preview exactly 15 seconds before the end. This also triggers the black frame detection.

@AbandonedCart
Copy link
Collaborator

The dreaded originals always have follow-up credits.

@AbandonedCart
Copy link
Collaborator

I think this one should be prioritized. A lot of the changes here will drastically improve performance and will also open a lot of potential for improving the automatic scans even further.

@rlauuzo rlauuzo marked this pull request as ready for review June 11, 2024 15:40
@jumoog
Copy link
Owner

jumoog commented Jun 11, 2024

whats a good anime for testing?

@AbandonedCart
Copy link
Collaborator

My go to has been Demon Slayer because the first episode of the season always puts the intro at the end . It has an intro, credits, and preview. The intro ends in a splash screen so it's easy to locate.

Copy link
Owner

@jumoog jumoog left a comment

Choose a reason for hiding this comment

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

tests are still working and code looks good.

LGTM

@Gauvino
Copy link

Gauvino commented Jun 14, 2024

Don't know about the code but been running this PR for 2 days without any problem

@jumoog
Copy link
Owner

jumoog commented Jun 14, 2024

is this ready?

@jumoog jumoog self-requested a review June 14, 2024 19:51
@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Jun 15, 2024

I guess so. I'm not entirely sure what the optimal way is to handle the lists for loading existing fingerprints and other data in the Chromaprint analyzer. That being said, the Chromaprint analyzer has some other areas that could be improved, like the silence detection, which seems a bit pointless in its current implementation. But those refinements are probably best left for another time.

@jumoog
Copy link
Owner

jumoog commented Jun 15, 2024

Yes, I think we can merge it in this form for now and make a new release.

@jumoog jumoog merged commit 9388f2a into master Jun 15, 2024
3 checks passed
@jumoog jumoog deleted the refactor-item-queue branch June 15, 2024 11:16
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

4 participants