Skip to content

Conversation

@kd7hto
Copy link

@kd7hto kd7hto commented Jan 6, 2021

Hello. Please review this code and consider it for the master branch. It includes some modifications to support the implementation of rfc 8643 (Optional SRTP or OSRTP). In addition to this code change, we also added the following xml change:

kd7hto added 3 commits January 6, 2021 11:04
Added implementation for Optional SRTP (OSRTP) as defined in rfc8643.
@signalwire-ci
Copy link

signalwire-ci bot commented Jan 6, 2021

@andywolk
Copy link
Contributor

andywolk commented Feb 2, 2021

@kd7hto

get_time
  test/test_aws.c(209):
    chk_eq_str: '20210106T184838Z' != '20210106T184839Z'

Here is the failing test:
https://github.com/signalwire/freeswitch/blob/master/src/mod/applications/mod_http_cache/test/test_aws.c#L209
and it is failing on:

fst_check_string_equals(time_stamp_test, time_stamp);

saying time_stamp_test does not match time_stamp.

I restarted Drone CI to see if that test fails again.
Purpose of the tests is to make sure that changes do not break anything.
So now we need to make sure that this PR does not break anything.

@kd7hto
Copy link
Author

kd7hto commented Feb 8, 2021

@andywolk so it looks like it passed the automation build and test. Is there something else I need to be doing to further this along? Do I need to get someone assigned to review the proposed changes?

@crienzo
Copy link
Member

crienzo commented Feb 13, 2021

@andywolk switch_core_media changes will not affect mod_http_cache.

@crienzo
Copy link
Member

crienzo commented Feb 13, 2021

Big problem with accepting changes to this part of FreeSWITCH is there is no evidence it is correct. Hard to validate by gut instinct.

@kd7hto
Copy link
Author

kd7hto commented Feb 15, 2021

@crienzo, @andywolk, thanks for taking a look at this. So if you've looked, the RFC is not a big one. We have several endpoints that have the ability to signal for OSRTP that I'm happy to schedule some time with you to do some testing if that helps.

The alternative as I see it is that we keep working with a one-off, however, we would very much like this to make it into the code base so that it is there for everyone in future releases.

Please let me know if I can help you with some testing with our endpoints.

Thank you.

Eugene Christensen

@kd7hto
Copy link
Author

kd7hto commented Mar 5, 2021

@andywolk @crienzo, please advise me on this. Is there no option for working forward to inclusion into the baseline product?

Thank you.

Eugene.

@crienzo
Copy link
Member

crienzo commented Mar 5, 2021

There is. We have quite a backlog at the moment, but can work to move forward on this.

@kd7hto
Copy link
Author

kd7hto commented Mar 5, 2021

Thank you.

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.

3 participants