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

Per request - here's to main | Significantly improve video resolution/quality using ESPCN_x4 model #704

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

saleweaver
Copy link

@saleweaver saleweaver commented Oct 7, 2024

Hi Kenneth,

here's the changes to main.

The other PR I had (you merged to experimental) had some MacOS fixes - should I create another one for main too? (If I saw it right, you didn't merge the stuff that destroyed MacOS support to main, so should be fine... :))

Greets!

Summary by Sourcery

Add a super-resolution frame processor using the ESPCN_x4 model to improve video quality. Introduce new command-line options for configuring super-resolution and image scaling factors, and update the README to reflect these changes.

New Features:

  • Introduce a new super-resolution frame processor using the ESPCN_x4 model to significantly enhance video resolution and quality.

Enhancements:

  • Add new command-line arguments for setting the super-resolution scale factor, enhancer upscale factor, and source image scaling factor to provide more control over image processing.

Documentation:

  • Update the README to include documentation for the new super-resolution frame processor and the additional command-line arguments.

Copy link
Contributor

sourcery-ai bot commented Oct 7, 2024

Reviewer's Guide by Sourcery

This pull request introduces a new super-resolution feature to enhance video quality using the ESPCN_x4 model. It adds a new frame processor option, updates the command-line interface, and implements the super-resolution functionality.

Sequence diagram for super-resolution process

sequenceDiagram
    participant User
    participant CLI
    participant SuperResolutionModel
    participant FrameProcessor
    User->>CLI: Provide input parameters
    CLI->>SuperResolutionModel: Initialize model
    SuperResolutionModel->>FrameProcessor: Process frames
    FrameProcessor->>SuperResolutionModel: Apply super-resolution
    SuperResolutionModel-->>FrameProcessor: Return enhanced frames
    FrameProcessor-->>CLI: Output processed frames
    CLI-->>User: Return enhanced video
Loading

File-Level Changes

Change Details Files
Added super-resolution as a new frame processor option
  • Updated the '--frame-processor' argument to include 'super_resolution' as a choice
  • Added new command-line arguments for super-resolution settings
  • Updated README.md to reflect new command-line options
modules/core.py
README.md
Implemented super-resolution functionality
  • Created a new file 'super_resolution.py' to handle super-resolution processing
  • Implemented a singleton class 'SuperResolutionModel' to manage the super-resolution model
  • Added functions for pre-check, pre-start, and processing frames with super-resolution
  • Integrated super-resolution processing into the existing frame processing pipeline
modules/processors/frame/super_resolution.py
Added new global variables for super-resolution and image scaling
  • Added 'enhancer_upscale_factor', 'source_image_scaling_factor', and 'sr_scale_factor' to globals
  • Updated 'parse_args' function to set these new global variables
modules/globals.py
modules/core.py
Improved image processing capabilities
  • Added function to upscale source images for better quality before face detection
  • Implemented separate processing functions for images and videos
modules/processors/frame/super_resolution.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @saleweaver - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider adding performance metrics or warnings for the super-resolution feature, especially for video processing on less powerful hardware.
  • The new command-line arguments are useful, but consider expanding the documentation to provide more detailed explanations and usage examples for these new features.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟡 Documentation: 2 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

modules/core.py Show resolved Hide resolved
modules/processors/frame/super_resolution.py Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@rotatorotator
Copy link

I tried pulling this yesterday and got a ctypes error (ImportError: cannot import name 'windll' from 'ctypes'). I am running Linux (Ubuntu).

@saleweaver
Copy link
Author

I tried pulling this yesterday and got a ctypes error (ImportError: cannot import name 'windll' from 'ctypes'). I am running Linux (Ubuntu).

You shouldn't be able to pull main than as well. That's an error in the repo, windll is only available on windows. I had it fixed before, I'll have a look sometime.

@rotatorotator
Copy link

Thanks for the quck reply, I am happy to test it if you found anything!

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