Skip to content

Conversation

@shogo4405
Copy link
Collaborator

Description & motivation

  • Added IPv6 support in the SRT protocol.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots:

@shogo4405 shogo4405 added the SRT Indicates that an issue, pull request, or discussion is concerns the SRT. label Nov 24, 2025
@shogo4405 shogo4405 requested a review from Copilot November 24, 2025 12:44
Copy link
Contributor

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 adds IPv6 support to the SRT protocol implementation by replacing the IPv4-specific sockaddr_in with a protocol-agnostic AddrInfo utility that uses getaddrinfo() for address resolution. This enables the library to handle both IPv4 and IPv6 addresses transparently.

Key Changes:

  • Introduced new AddrInfo struct that wraps getaddrinfo() for protocol-agnostic address resolution
  • Updated SRT socket connection/binding logic to use the new resolver pattern
  • Removed deprecated IPv4-only sockaddr_in extension that used gethostbyname()

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
SRTHaishinKit/Sources/Util/AddrInfo.swift New utility struct implementing address resolution using getaddrinfo() with support for both IPv4 and IPv6
SRTHaishinKit/Sources/SRT/SRTSocketURL.swift Updated remote and local properties to return AddrInfo instead of sockaddr_in
SRTHaishinKit/Sources/SRT/SRTSocket.swift Refactored connection/binding logic to use AddrInfo.resolve() with proper error handling for all three SRT modes
SRTHaishinKit/Sources/Extension/sockaddr_in+Extension.swift Removed IPv4-only extension that relied on deprecated gethostbyname()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +13 to +17
@discardableResult
func resolve<R>(_ flags: Int32, lambda: (UnsafePointer<sockaddr>, Int32) throws -> R?) throws -> R {
var hints = addrinfo(
ai_flags: flags,
ai_family: AF_UNSPEC,
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

[nitpick] Setting ai_family to AF_UNSPEC allows both IPv4 and IPv6, but the flags parameter doesn't validate which protocols are actually requested. Consider documenting this behavior or adding a parameter to allow callers to specify protocol preferences (e.g., IPv4-only, IPv6-only, or both).

Suggested change
@discardableResult
func resolve<R>(_ flags: Int32, lambda: (UnsafePointer<sockaddr>, Int32) throws -> R?) throws -> R {
var hints = addrinfo(
ai_flags: flags,
ai_family: AF_UNSPEC,
/**
Resolves the host and port to a socket address.
- Parameters:
- flags: Flags for address resolution.
- family: Protocol family to use (AF_INET for IPv4, AF_INET6 for IPv6, AF_UNSPEC for both). Defaults to AF_UNSPEC.
- lambda: Closure to process each resolved address.
- Returns: The result of the closure for the first matching address.
- Throws: Error.failedToGetaddrinfo or Error.failedToResolve.
*/
@discardableResult
func resolve<R>(_ flags: Int32, family: Int32 = AF_UNSPEC, lambda: (UnsafePointer<sockaddr>, Int32) throws -> R?) throws -> R {
var hints = addrinfo(
ai_flags: flags,
ai_family: family,

Copilot uses AI. Check for mistakes.
guard let local = url.local else {
throw Error.invalidArgument("missing local url")
}
return try remote.resolve(AI_PASSIVE | AI_ADDRCONFIG) { remotename, remotelen in
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

Using AI_PASSIVE flag for remote address resolution in rendezvous mode is incorrect. AI_PASSIVE is intended for addresses that will be used with bind(), not connect() or rendezvous connections. The remote address should use AI_ADDRCONFIG only, while only the local address should use AI_PASSIVE.

Suggested change
return try remote.resolve(AI_PASSIVE | AI_ADDRCONFIG) { remotename, remotelen in
return try remote.resolve(AI_ADDRCONFIG) { remotename, remotelen in

Copilot uses AI. Check for mistakes.
Comment on lines +159 to +160
return try local.resolve(AI_PASSIVE | AI_ADDRCONFIG) { localname, locallen in
let status = srt_rendezvous(socket, localname, locallen, remotename, remotelen)
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

The parameters to srt_rendezvous are in the wrong order. According to SRT API documentation, the function signature should have local address parameters first, then remote address parameters. This line passes them as (socket, local, local_len, remote, remote_len) but based on the old code structure and typical SRT API patterns, verify the correct parameter order.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

SRT Indicates that an issue, pull request, or discussion is concerns the SRT.

Development

Successfully merging this pull request may close these issues.

2 participants