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

Core: Minor Bug Fixes #406

Merged
merged 17 commits into from
Jun 19, 2024
Merged

Core: Minor Bug Fixes #406

merged 17 commits into from
Jun 19, 2024

Conversation

abhiTronix
Copy link
Owner

@abhiTronix abhiTronix commented Jun 18, 2024

Brief Description

This PR encompasses a series of enhancements and bug fixes across multiple components of the project, aimed at improving functionality, optimizing performance, and enhancing user experience.

Key Changes: 🆕

Here are the main changes included:

WebGear_RTC:

  • Optimized peer connection closure to avoid redundant closures.
  • Reduced unnecessary logging by only logging ICE connection state changes when they are not in a "failed" state.

StreamGear:

  • Enhanced stream copy support in Single Source mode (Fixes [Proposal]: -vcodec copy HLS #396)
    • Ignored stream copy parameter in Real-time Frames Mode or Custom Streams with appropriate warnings.
    • Updated -acodec handling:
      • Default to aac for Custom Streams.
      • Use stream copy (-acodec copy) for input video’s audio when Custom Streams are disabled.
    • Refined -livestream parameter usage to Real-time Frames Mode only.
    • Adjusted video and audio bitrate assignment to skip when stream copy is enabled.
    • Improved log message for -clear_prev_assets parameter.
  • Moved handle streaming format to beginning to fix 'StreamGear' object has no attribute '_StreamGear__format' bug.

CamGear:

  • Removed GStreamer support check.
  • Improved readability of livestream warning logs.

WriterGear:

  • Improved error handling in execute_ffmpeg_cmd method:
    • Raised ValueError with descriptive messages for BrokenPipeError or IOError.
    • Updated error handling per PEP 409 to preserve original exception context or suppress it based on logging settings.

NetGear: Updated parameters and documentation (Fixes #390)

  • Added warning log for potential issues with flag=1 (NOBLOCK).
  • Noted that track option is ignored when copy=True.

Core:

  • Improved exception handling for module imports.
    • Updated import_dependency_safe in helper.py:
      • dded specific handling for ModuleNotFoundError.
      • Included original exception in ImportError for better error tracing.
      • Enhanced logging to include exception traceback when error is set to "log".
    • Enhanced import_core_dependency in __init__.py:
      • Added specific handling for ModuleNotFoundError.
      • Included original exception in ImportError for better error tracing.

Maintenance:

  • Refactored colorspace handling in videocapture gears
    • Logged a warning and discarded invalid colorspace values instead of raising an exception.
    • Consolidated colorspace logging into a single line using a ternary operation.
  • Simplified code using short-circuiting.
  • Corrected a typo in comments.
  • Removed unnecessary parentheses and type checks.
  • Removed unused imports.

Docs:

  • Enhanced StreamGear documentation:
    • Added a tip box on benefits of using stream copy (-vcodec copy) for faster HLS/DASH transcoding.
    • Highlighted limitations of stream copy, including incompatibility with Real-time Frames Mode and Custom Streams.
    • Clarified automatic audio stream copy (-acodec copy) usage with input video’s audio stream.
  • Changed default value of copy to True in NetGear API documentation.
  • Fixed typos, formatting, code highlighting, and grammar issues.

CI:

  • Enhanced WebGear RTC tests
  • Updated NetGear unit tests to reflect the new default for copy.
  • Fixed simplejpeg and opencv not compatible with numpy==2.x.x versions.
    • Pinned numpy<2.0.0 in all CI envs.
  • Added test cases for import_dependency_safe function to validate different scenarios and error handling in import_dependency_safe.
    • Ensured coverage for raise, log, silent, and unknown error types.
  • Fixed invalid escape sequence in testcase string.
  • Fixed python environment bugs in appveyor.yml.
  • Removed pinned cryptography==38.0.4 dependency.

Requirements / Checklist

Related Issue

#390
#396

Context

This PR is part of ongoing efforts to improve the overall functionality, performance, and usability of the project, addressing both user-reported issues and opportunities for internal optimization.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

- ⚡️ Only close peer connections that are not already in the "closed" state.
- 🎨 Logged the ICE connection state change only when it's not in the "failed" state, reducing unnecessary logging.
- 👷 Added a pytest fixture `event_loop_policy` to set the WindowsSelectorEventLoopPolicy on Windows platforms.
- ⚡️Replace the use of `async_asgi_testclient` with `httpx.AsyncClient` and `httpx.ASGITransport` for testing the ASGI application.
- 🥅 Update the test cases to use the `httpx.AsyncClient` correctly:
   - Use the `content` parameter instead of `data` when sending POST requests with JSON payloads.
   - Use the `post` method instead of `get` when sending WebRTC offers, as `get` do not support `content` parameter.
- 🔥 Remove the `pytest.mark.skipif` conditions related to Python version 3.11 and above, as the compatibility issues have been addressed.
- 🗑️ Add the `pytest.mark.asyncio(scope="module")` marker to the test functions to ensure proper handling of asynchronous tests.
…ixes #396)

- ♻️ Ignore the stream copy parameter if Real-time Frames Mode or Custom Streams are enabled, and log appropriate warnings.
- ⚡️Updated the handling of the `-acodec` parameter:
     - Use the default `aac` codec for Custom Streams.
     - Use stream copy (`-acodec copy`) for the input video's audio stream if Custom Streams are not enabled.
- ♻️ Refactor the handling of the `-livestream` parameter to ensure it is only enabled for the Real-time Frames Mode.
- ♻️ Refactor the video and audio bitrate assignment to skip the assignment when stream copy is enabled.
- 🔊 Updated log message for `-clear_prev_assets` parameter.
- ✏️ Fix a typo in comments.

Docs:
- 📝 Add a new tip box explaining the benefits of using stream copy (`-vcodec copy`) in the Single Source Mode for faster transcoding of HLS/DASH streams.
- 💬 Highlight the limitations of stream copy, such as incompatibility with Real-time Frames Mode and Custom Streams, which require re-encoding of frames.
- 💬 Clarify that the audio stream copy (`-acodec copy`) is automatically applied when using the input video's audio stream.
- 🎨 Fixed various issues like typos, formatting errors, code highlighting issues, and grammar inconsistencies.
- ⚡️ Instead of raising an exception, log a warning message and discard the invalid colorspace value.
- 🎨 Consolidate the colorspace logging statement to a single line using a ternary operation.

CamGear API
- 🔥 Remove the check for GStreamer support, as it is not being used currently (marked as a TODO).
- 🔊 Improve the readability of the livestream warning log.

Maintenance
- 🎨 Applied short-circuiting to simplify code across various APIs.
- 🎨 Remove unnecessary parentheses and unnecessary type checks.
- 🔧 Removed unused imports
- 🎨 Instead of raising a generic `ValueError` exception, the method now raises a `ValueError` with a more descriptive error message when encountering a `BrokenPipeError` or `IOError`.
- 🥅 The error handling has been updated to follow the recommendations of PEP 409 - Suppressing exception context. This ensures that the original exception context is preserved when re-raising the `ValueError`.
- ⚡️ If logging is enabled (`self.__logging` is True), the `ValueError` is raised with the suppressed context (`from None`), effectively discarding the original exception context.
- ⚡️ If logging is disabled, the `ValueError` is raised with the original exception context (`from e`), where `e` is the original `OSError` or `IOError` exception.
- 🥅 Added a warning message in the `NetGear` class to log potential issues when `flag=1` (NOBLOCK) is set, which may cause NetGear to not terminate gracefully.
- 🔊 Included an informational log message indicating that the `track` option will be ignored when `copy=True` is defined.
- 📝 Changed the default value of the `copy` option from `False` to `True` across various documentation files for NetGear.

👷 CI: Updated unit tests to reflect the new default value for the `copy` option.

✏️ Docs: Fixed few typos.
@abhiTronix abhiTronix added BUG 🐛 Vidgear api's error, flaw or fault WORK IN PROGRESS 🚧 currently been worked on. DOCS 📜 Issue/PR is related to vidgear docs. PENDING TESTS 🧪 Waiting for CI tests to complete successfully. labels Jun 18, 2024
@abhiTronix abhiTronix added this to the v0.3.3 milestone Jun 18, 2024
@abhiTronix abhiTronix self-assigned this Jun 18, 2024
…treamGear' object has no attribute '_StreamGear__format' bug.
…x.x` versions.

- 📌 Pinned `numpy<2.0.0` in all CI envs.
- ⏪️ Reverted testing simplejpeg import
- ⚡️ Updated `import_dependency_safe` in `helper.py`:
  - 🥅 Added specific handling for `ModuleNotFoundError`.
  - 🧑‍💻 Included original exception in `ImportError` for better error tracing.
  - 🔊 Enhanced logging to include exception traceback when error is set to "log".

- ⚡️ Enhanced `import_core_dependency` in `__init__.py`:
  - 🥅 Added specific handling for `ModuleNotFoundError`.
  - 🧑‍💻 Included original exception in `ImportError` for better error tracing.
.
- ⚡️ Included various test cases to validate different scenarios and error handling in `import_dependency_safe`.
- ☔ Ensured coverage for `raise`, `log`, `silent`, and unknown error types.
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 89.04110% with 8 lines in your changes missing coverage. Please review.

Project coverage is 93.57%. Comparing base (d4243ab) to head (0d9071e).

Files Patch % Lines
vidgear/gears/writegear.py 0.00% 4 Missing ⚠️
vidgear/gears/asyncio/webgear_rtc.py 33.33% 2 Missing ⚠️
vidgear/gears/screengear.py 90.90% 1 Missing ⚠️
vidgear/gears/streamgear.py 95.45% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           testing     #406      +/-   ##
===========================================
- Coverage    93.85%   93.57%   -0.29%     
===========================================
  Files           15       15              
  Lines         3092     3080      -12     
===========================================
- Hits          2902     2882      -20     
- Misses         190      198       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abhiTronix abhiTronix merged commit e533553 into testing Jun 19, 2024
11 checks passed
@abhiTronix abhiTronix deleted the development branch June 19, 2024 19:47
@abhiTronix abhiTronix added SOLVED 🏁 This issue/PR is resolved now. Goal Achieved! and removed WORK IN PROGRESS 🚧 currently been worked on. PENDING TESTS 🧪 Waiting for CI tests to complete successfully. labels Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG 🐛 Vidgear api's error, flaw or fault DOCS 📜 Issue/PR is related to vidgear docs. SOLVED 🏁 This issue/PR is resolved now. Goal Achieved!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal]: -vcodec copy HLS Exception in thread NetGear, zmq.error.Again: Resource temporarily unavailable
1 participant