Skip to content

Conversation

mondain
Copy link
Member

@mondain mondain commented Jun 19, 2025

Fixes and simplifies RTMPS client use; does not require CA signed stores and uses standard java.net system properties.
Tests module was repaired and made a proper mvn participant
Test files moved to proper location
BBB test flv added for source content

@mondain mondain requested review from Andy--S and Copilot June 19, 2025 15:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aims to fix and simplify the RTMPS client usage while repairing and organizing the test modules. Key changes include adjustments to logging levels and logger configurations, refactoring of client connection and TLS/keystore handling logic, and updates to test files and project module configurations.

Reviewed Changes

Copilot reviewed 25 out of 66 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/src/test/resources/logback-test.xml Lowered the root log level from TRACE to DEBUG and added logger definitions for various components.
tests/src/test/resources/Readme.md Added a new section heading and formatted the test properties information.
tests/src/test/java/org/red5/net/websocket/WebSocketServerTest.java Updated the cookie generation method to use insecure().nextAlphanumeric.
tests/src/test/java/org/red5/client/net/rtmp/TwitchConnectTest.java & FBLiveConnectTest.java Updated test methods and parameter names to use streamKey consistently.
tests/src/test/java/org/red5/client/PublisherTest.java, SharedObjectClient.java, RemoteSOTest.java, ClientTest.java Changed random string generation methods for consistency and minor refactoring in test packages.
pom.xml and module-specific pom.xml files Added and re-ordered modules; removed unnecessary dependency declarations.
client/src/main/java/org/red5/client/net/rtmps/RTMPSClient.java Refactored TLS handling, keystore/truststore password management, and certificate retrieval; added new constructors and system property handling.
Other client files (e.g. CertificateGrabber, P12StoreManager) Added utilities for certificate retrieval and truststore management.

Copy link
Contributor

@Andy--S Andy--S left a comment

Choose a reason for hiding this comment

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

Following copilot's suggestion, maybe only log out the passwords if they fail, and also use a final isDebug boolean to wrap that logger statement.

  1. if the password works, there is nothing to debug.
  2. The logback setting of final boolean isDebug cant be changed at run time.

Or just remove the password logout now that youve seen the setup working.

@mondain mondain merged commit d398204 into main Jun 19, 2025
@mondain mondain deleted the bug/rtmpclient branch June 19, 2025 17:05
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