TEL-23701: Voipmonitor rebase and updated config (29.1.4 -> 2025.04.4) #7
Conversation
There was a problem hiding this comment.
Great job with this!
It can be done separately, but our list of commits can be improved further:
- init.d seems to be ignored - we actually use https://github.com/dialpad/servers/blob/master/ansible/roles/te/files/voipmonitor.init. We should put that in the repo, and drop our previous changes to init.d.
- I think "pcap still missing audio on mpv calls" can be dropped - it now adds an enum value that's never used
- "Cherry picked changes from squashed rebase commits" and the first 2 file changes in "Removing the automatic printing of stacktrace that's hanging voipmon; removed is_video variable;" shouln't exist - their changes are actually part of "Need flag to disable video RTP capture (voipmonitor)"
config/voipmonitor.conf
Outdated
| #pauserecordingheader = MyCustomPauseHeader | ||
| # Whether video RTP packets should include the payload or just the header. | ||
| # We disable video payload recording for TEs with too little disk space | ||
| {% if not is_container_build %} |
There was a problem hiding this comment.
This (and 2 other occurrences in this file) is an Ansible thing, so it doesn't make sense in a voipmon config - voipmon can't read this file.
A better way to achieve what we're trying to do, would be to have
record_video_payload = no
here, and have Ansible update the config before starting voipmon, if necessary.
Now I know that we don't actually use this file, but as I said in the other review, I think we should.
calltable.h
Outdated
| @@ -457,7 +457,8 @@ struct ip_port_call_info { | |||
| enum eTypeAddr { | |||
| _ta_base, | |||
| _ta_natalias, | |||
| _ta_sdp_reverse_ipport | |||
| _ta_sdp_reverse_ipport, | |||
| _ta_base_video, | |||
There was a problem hiding this comment.
I don't think the residual of the commit f6a7810 is needed anymore.
It's adding an _ta_base_video that is not used anymore.
calltable.cpp
Outdated
| @@ -132,6 +132,7 @@ extern int opt_printinsertid; | |||
| extern int opt_cdronlyanswered; | |||
| extern int opt_cdronlyrtp; | |||
| extern int opt_newdir; | |||
| extern int opt_video_recording; | |||
There was a problem hiding this comment.
I don't think that this commit 2f16db1 is needed anymore.
The record_video_payload config has now been superseded by the savertp_video config, and this patch can be dropped.
configure.in
Outdated
| if test $LIBSSH_VERSION -lt 8; then | ||
| LIBSSH_8=0 | ||
| fi | ||
|
|
There was a problem hiding this comment.
According to the changelogs, the code dropped its dependency on libssh, so I think we can drop this commit 2618dfb as well.
25.5.1 21.08.2019
=================
- add audio conversion (get rid of sox) [VG-1883]
- fix audio sync when there are mark bit in every packets [VS-957]
- remove libssh code and library dependency (not needed anymore) [VS-947]
There was a problem hiding this comment.
I'll add the commits to remove to (https://dialpad.atlassian.net/browse/TEL-24608) and tackle these after the rebase is through QA & rollout. Corey pointed out we might want to have revert commits for some changes we want to keep history for. I'll remove the dead code in a commit as a temporary measure for the rebase though.
codecs.h
Outdated
| @@ -60,6 +60,7 @@ | |||
| #define PAYLOAD_TELEVENT 400 | |||
|
|
|||
| #define PAYLOAD_VIDEO 10000 | |||
| #define VP8 10001 | |||
There was a problem hiding this comment.
We can delete this, as it is part of the custom Dialpad change or the record_video_payload config option that is no longer needed.
codecs.h
Outdated
| @@ -60,7 +60,7 @@ | |||
| #define PAYLOAD_TELEVENT 400 | |||
|
|
|||
| #define PAYLOAD_VIDEO 10000 | |||
| #define VP8 10001 | |||
| #define PAYLOAD_VP8 10001 | |||
There was a problem hiding this comment.
We can delete PAYLOAD_VP8 as it's no longer needed.
|
[NIT:] We're rebasing over 2025.04.4 dated 14.04.2025 So title should read: |
kavindail
changed the title
|
Stored copies of pre rebased master at master_20250605 & voipmonitor-v29.1.4-rebase. |
Summary: Rebasing Voipmon from a 2022 version to the latest master (2025). Enabled config options to make pcap analysis / voipmonitor crashes(coredumps) have more information.
These changes also rewrite our history to remove the squashed commits from previous rebases as well as some unnecessary outdated commits. This was purposefully done to make rebasing in the future go much smoother, decreasing our footprint in the Voipmon code.
Rollout plan: These changes merging are contingent on the staging/production testing working as expected. Rollout plan
Tests Done: Tested calling on UVStaging and UCStaging in multi person conferences, analyzing the pcaps directly from the TE. Also ensured that audio files & pcaps were being generated correctly when FST issues were created. Compared pcap sizes before & after rebase, cpu usage, memory usage, etc.