SRTP: Consolidate Srtp{Audio,Video}InfoParams#824
Conversation
They are identical.
|
There is so much code duplication in SRTP logic! I'm working on reducing it. This is just the first step. |
|
@jeannotlanglois Please review. |
Correct - in 2016+2017 when I was doing the implementation I initially thought that differences could exist between AUDIO and VIDEO media parameters. But now after several years having seen many practical uses of various kinds of scenarios I can confirm that this is not the case. In 2016 I was really working on getting something basic working and because of all the extensive testing I kept the AUDIO and VIDEO parameters separate to make sure I wouldn't be mixing between any of the two when debugging. I really was planning to do a few PRs of refactoring in 2022-2024 but it didn't happen due to some employment changes that affected my schedule. After reviewing your code carefully I am ok with your changes. This is good/safe as far as I can tell, so I don't see the need for any retesting. Approved... AFAIK this could be merged. As a note: I would suggest making a new SIPP 3.7.6 bugfix release soon (with this patch merged to master) so that the issue seen recently with unterminated arrays can be made publicly available ( as I believe those have not been picked up in a new release yet ). Specially since testing indicates that SRTPCHECK is in a healthy state now. I'll return in January after my holiday break. Looks like I might only be available a few hours one day every week or two for a while in 2026. Thanks for taking your time to slowly help cleanup duplication, as I wish I would have been able to do it myself. Going slowly and steady with retesting is the key. Happy Holidays! |
2 participants
They are identical.