-
Notifications
You must be signed in to change notification settings - Fork 18
Feature: refactor DTLS to merge it into tls_openssl.c #18
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 refactors the DTLS implementation by merging it with the existing TLS code, introducing shared data structures and functions and integrating DTLS support into the WHIP muxer.
- Introduces new DTLS fields and enum in the TLSShared structure in tls.h.
- Modifies connection and protocol selection logic in tls.c to differentiate between TCP and UDP usage for TLS/DTLS.
- Updates related modules (srtp, protocols, http, avio, allformats, Makefile, and documentation) to support the merged DTLS/TLS implementation.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
libavformat/tls.h | Added DTLS-specific fields, enum DTLSState, and new AV options. |
libavformat/tls.c | Updated connection logic to select UDP for DTLS and added functions for DTLS. |
libavformat/srtp.h | Changed SRTPContext to a typedef. |
libavformat/protocols.c | Declared the new ff_dtls_protocol. |
libavformat/http.h & http.c | Added function(s) for new HTTP location retrieval. |
libavformat/avio.c | Extended warning message to check for dtls. |
libavformat/allformats.c | Adjusted muxer declarations to include WHIP muxer support. |
libavformat/Makefile | Integrated new object files for the WHIP muxer build. |
doc/muxers.texi | Provided documentation for the new WHIP muxer. |
configure | Updated configuration to support the whip_muxer dependency on openssl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this PR, and it works as expected. Thanks for your good work.
2fc886e
to
cb68e9b
Compare
0. Version 1. 1. The WHIP muxer has been renamed and refined, with improved logging context and error messages for SSL, DTLS, and RTC. 2. Magic numbers have been replaced with macros and extracted to functions, and log levels have been altered for better clarity. 3. DTLS curve list has been updated, and SRTP profile names have been refined for FFmpeg and OpenSSL. 4. ICE STUN magic number has been refined, and RTP payload types have been updated based on Chrome's definition. 5. Fixed frame size has been refined to rtc->audio_par->frame_size, and h264_mp4toannexb is now used to convert MP4/ISOM to annexb. 6. OPUS timestamp issue has been addressed, and marker setting has been corrected after utilizing BSF. 7. DTLS handshake and ICE handling have been optimized for improved performance, with a single handshake timeout and server role to prevent ARQ. 8. DTLS BIO callback has been implemented for packet fragmentation, and MTU settings have been refined using SSL_set_mtu and DTLS_set_link_mtu. 9. Consolidated ICE request/response handling and DTLS handshake into a single function, and fixed OpenSSL build errors to work with Pion. ------ Co-authored-by: winlin <[email protected]> Co-authored-by: yangrtc <[email protected]> Co-authored-by: cloudwebrtc <[email protected]> Co-authored-by: Haibo Chen <[email protected]> Signed-off-by: Steven Liu <[email protected]>
Signed-off-by: Jack Lau <[email protected]>
Signed-off-by: Jack Lau <[email protected]> Abstract dtls as ffmpeg protocol Since i make the dtls as a ffmpeg protocol, we need init it use ffurl_oepn after ICE binding. But before that, SDP need the fingerprint of cert So i implement these gen or read in separated function and can be called by whip because we need obtain fingerprint for sdp exchange. So we need pass the string of key and cert into dtls from whip. debug: set info callback for tls_openssl dtls: refactor the logic of bio make the dtls use custom bio rather than callback to match the tls implementation dtls: add option that re-use udp from demuxer/muxer integrate print_ssl_error, read, write as same as implementation in tls remove openssl_dtls_state_trace, opessl_ssl_get_error. implemente dtls_handshake function. pass patchcheck fix tailing whitespace, missing av_cold, x==0 can be simplified, forget use av_log, use snprintf instead of sprintf, add braces for if else (Don’t wrap single-line blocks in braces. Use braces only if there is an accompanying else statement.) Signed-of-by: Jack Lau <[email protected]>
handshake spells error Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Jack Lau <[email protected]>
cb68e9b
to
a9a670c
Compare
improve the api name like url_read_all -> ff_url_read_all. fix av_log in tls_openssl. use thread-safe function openssl_get_error instead of ERR_error_string(ERR_get_error(), NULL) in part of tls_openssl. In other case, use the ERR_error_string(ERR_get_error(), NULL) when there is no TLSContext.
This patch make the whip muxer depends dtls protocol, and dtls protocol depends openssl for now. Signed-off-by: Jack Lau <[email protected]>
94774ad
to
936f3f1
Compare
Signed-off-by: Jack Lau <[email protected]>
This patch is based on #1 but do meangful refactor work. The below is details
You can follow the usage to test this patch.
This PR has been squashed into a single commit on the patch/whip/v2 branch for easier submission to the FFmpeg community for code review.