pjsua: Add audio play/rec commands to CLI framework #4723

vasilevalex · 2025-12-29T14:29:15Z

Summary

Add commands to dynamically start/stop audio file playback and recording during calls. These commands are available in the CLI framework (enabled with --use-cli option) via telnet front-end.
These commands complement the existing --auto-play and --auto-rec startup options.

Note: These commands are not added to the legacy console menu.

New Commands

Under the audio command group:

audio play_start <filename> - Start playing a WAV file to the current call
audio play_stop - Stop playback and reconnect microphone
audio rec_start <filename> - Start recording the call to a file
audio rec_stop - Stop recording

Features

Commands can be issued before or during a call
If issued before call setup, playback/recording is queued and starts automatically when the call is established
Playback disconnects the microphone (like --auto-play)
Recording captures both call audio and microphone
Resources are automatically cleaned up when the associated call ends
Commands are blocked if --auto-play/--auto-rec startup options are active (returns error)

Behavior with Multiple Calls

These commands use one-shot semantics, unlike --auto-play/--auto-rec which persist across all calls:

Playback/recording attaches to a single call only
If command is queued before any call, it attaches to the first call that becomes active
If a second call starts while playback/recording is active on the first call, the second call is not affected
When the attached call ends, playback/recording stops and the queue is cleared
To play/record on a new call, the command must be issued again

Copilot

Pull request overview

This PR adds CLI commands for dynamic audio playback and recording control during calls. The new commands (play_start, play_stop, rec_start, rec_stop) are available via the CLI framework (enabled with --use-cli) and support queuing operations before calls are established, with automatic activation when calls become active.

Key Changes

New state management fields for dynamic playback/recording with one-shot semantics (attaches to a single call)
CLI command handlers with queuing support that activates automatically when calls establish
Automatic cleanup of playback/recording resources when the associated call ends

Reviewed changes

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

File	Description
pjsip-apps/src/pjsua/pjsua_app_common.h	Adds state tracking fields for dynamic player and recorder (IDs, ports, call association, active flags, filenames)
pjsip-apps/src/pjsua/pjsua_app_config.c	Initializes new dynamic playback/recording state variables to invalid/inactive states
pjsip-apps/src/pjsua/pjsua_app_cli.c	Implements CLI command handlers and helper functions (start/stop for playback/recording), adds XML command definitions for the audio command group
pjsip-apps/src/pjsua/pjsua_app.c	Adds cleanup logic in call disconnect handler and queued activation logic in audio state callback

Copilot · 2025-12-30T06:56:17Z

pjsip-apps/src/pjsua/pjsua_app_cli.c

+    pj_ansi_strncpy(app_config.dyn_play_filename, filename, PJ_MAXPATH);
+    app_config.dyn_play_filename[PJ_MAXPATH-1] = '\0';
+    app_config.dyn_player_active = PJ_TRUE;


The filename should be validated before being copied. There's no check to ensure the filename is not empty or NULL, which could lead to issues. Consider validating that the filename is not empty and is a valid file path before storing it and setting the active flag.

The filename is already validated at the caller:

argc < 2 check ensures filename argument is provided (not empty, not NULL)

slen >= PJ_MAXPATH check ensures it's not too long

pjsua_player_create() will also return error for invalid/empty filenames
The project doesn't validate file paths upfront - this is consistent with how --auto-play and other file operations work. The file is validated when pjsua_player_create() tries to open it via pjmedia_wav_player_port_create(). If the file doesn't exist or is invalid, an error is returned and logged ("Unable to open file for playback")

Copilot · 2025-12-30T06:56:18Z

pjsip-apps/src/pjsua/pjsua_app_cli.c

+    /* Check if playback already active */
+    if (app_config.dyn_player_active) {
+        PJ_LOG(3, (THIS_FILE, "Playback already active, stopping first..."));
+        stop_playback();


When stop_playback fails on line 1399, the error status is not checked. If stop_playback returns an error, the function continues and may leave resources in an inconsistent state. Consider checking the return status and handling the error appropriately.

Suggested change

stop_playback();

status = stop_playback();

if (status != PJ_SUCCESS) {

PJ_PERROR(1, (THIS_FILE, status,

"Failed to stop existing playback"));

return status;

}

The stop_playback() error is already logged internally (line 1521), and the state is always cleaned up regardless of the result. pjsua_player_destroy() can only fail on assertions (invalid ID or player already destroyed)
Returning early here would prevent the user from starting a new playback, I think showing the warning (already done) and continuing is the ok behavior. But if you think, it should fail, I can use suggested error processing.

Copilot · 2025-12-30T06:56:18Z

pjsip-apps/src/pjsua/pjsua_app_cli.c

+    /* Check if recording already active */
+    if (app_config.dyn_rec_active) {
+        PJ_LOG(3, (THIS_FILE, "Recording already active, stopping first..."));
+        stop_recording();


When stop_recording fails, the error status is not checked, similar to the issue in start_playback. The function continues and may leave resources in an inconsistent state. Consider checking the return status and handling the error appropriately.

Suggested change

stop_recording();

status = stop_recording();

if (status != PJ_SUCCESS) {

PJ_PERROR(1,(THIS_FILE, status,

"Error stopping current recording before starting "

"dynamic recording"));

return status;

}

Copilot · 2025-12-30T06:56:18Z

pjsip-apps/src/pjsua/pjsua_app.c

+                PJ_LOG(3, (THIS_FILE, "Dynamic recording auto-started"));
+            } else {
+                PJ_LOG(2, (THIS_FILE, "Failed to auto-start recording: %d", status));
+                app_config.dyn_rec_active = PJ_FALSE;


When auto-start of recording fails and dyn_rec_active is set to FALSE (line 500), the dyn_rec_filename is not cleared. This creates an inconsistent state. The filename should be cleared along with the active flag to maintain consistency.

Suggested change

app_config.dyn_rec_active = PJ_FALSE;

app_config.dyn_rec_active = PJ_FALSE;

app_config.dyn_rec_filename[0] = '\0';

Copilot · 2025-12-30T06:56:19Z

pjsip-apps/src/pjsua/pjsua_app.c

+        /* Dynamic recording - start if queued */
+        if (app_config.dyn_rec_active &&
+            app_config.dyn_rec_id == PJSUA_INVALID_ID &&
+            app_config.dyn_rec_filename[0] != '\0')
+        {
+            pj_status_t status;
+            pjsua_recorder_id rec_id;
+            pjsua_conf_port_id rec_port;
+            pj_str_t rec_file;
+
+            /* Create recorder */
+            rec_file = pj_str(app_config.dyn_rec_filename);
+            status = pjsua_recorder_create(&rec_file, 0, NULL, 0, 0, &rec_id);
+            if (status == PJ_SUCCESS) {
+                rec_port = pjsua_recorder_get_conf_port(rec_id);
+
+                /* Connect call to recorder */
+                pjsua_conf_connect(call_conf_slot, rec_port);
+
+                /* Connect microphone to recorder */
+                pjsua_conf_connect(0, rec_port);
+
+                /* Save state */
+                app_config.dyn_rec_id = rec_id;
+                app_config.dyn_rec_port = rec_port;
+                app_config.dyn_rec_call = ci->id;
+
+                PJ_LOG(3, (THIS_FILE, "Dynamic recording auto-started"));
+            } else {
+                PJ_LOG(2, (THIS_FILE, "Failed to auto-start recording: %d", status));
+                app_config.dyn_rec_active = PJ_FALSE;
+            }


This code for creating and connecting the recorder is duplicated from start_recording in pjsua_app_cli.c (lines 1608-1634). Consider extracting this logic into a shared helper function to reduce code duplication and improve maintainability.

Copilot · 2025-12-30T06:56:20Z

pjsip-apps/src/pjsua/pjsua_app_cli.c

+    pj_ansi_strncpy(app_config.dyn_rec_filename, filename, PJ_MAXPATH);
+    app_config.dyn_rec_filename[PJ_MAXPATH-1] = '\0';
+    app_config.dyn_rec_active = PJ_TRUE;


The same validation and consistency issue exists here as in start_playback. The filename should be validated before being copied, and there's no check to ensure it's not empty or NULL.

Copilot · 2025-12-30T06:56:20Z

pjsip-apps/src/pjsua/pjsua_app_cli.c

+    /* Get call info */
+    status = pjsua_call_get_info(call_id, &call_info);
+    if (status != PJ_SUCCESS) {
+        PJ_LOG(2, (THIS_FILE, "Failed to get call info"));


After get_call_info fails and returns an error status, the function leaves dyn_rec_active set to TRUE and dyn_rec_filename populated, creating an inconsistent state. The function should reset these fields before returning an error to maintain consistency.

Suggested change

PJ_LOG(2, (THIS_FILE, "Failed to get call info"));

PJ_LOG(2, (THIS_FILE, "Failed to get call info"));

/* Reset dynamic recording state on failure to maintain consistency */

app_config.dyn_rec_active = PJ_FALSE;

app_config.dyn_rec_filename[0] = '\0';

Copilot · 2025-12-30T06:56:20Z

pjsip-apps/src/pjsua/pjsua_app.c

+                connect_sound = PJ_FALSE;
+            } else {
+                PJ_LOG(2, (THIS_FILE, "Failed to auto-start playback: %d", status));
+                app_config.dyn_player_active = PJ_FALSE;


When auto-start of playback fails and dyn_player_active is set to FALSE (line 550), the dyn_play_filename is not cleared. This creates an inconsistent state similar to the recording case. The filename should be cleared along with the active flag to maintain consistency.

Suggested change

app_config.dyn_player_active = PJ_FALSE;

app_config.dyn_player_active = PJ_FALSE;

app_config.dyn_play_filename[0] = '\0';

Copilot · 2025-12-30T06:56:21Z

pjsip-apps/src/pjsua/pjsua_app.c

+        /* Dynamic playback - start if queued */
+        if (app_config.dyn_player_active &&
+            app_config.dyn_player_id == PJSUA_INVALID_ID &&
+            app_config.dyn_play_filename[0] != '\0')
+        {
+            pj_status_t status;
+            pjsua_player_id player_id;
+            pjsua_conf_port_id player_port;
+            pj_str_t play_file;
+
+            /* Create player */
+            play_file = pj_str(app_config.dyn_play_filename);
+            status = pjsua_player_create(&play_file, 0, &player_id);
+            if (status == PJ_SUCCESS) {
+                player_port = pjsua_player_get_conf_port(player_id);
+
+                /* Connect player to call */
+                pjsua_conf_connect(player_port, call_conf_slot);
+
+                /* Disconnect microphone (like auto-play) */
+                pjsua_conf_disconnect(0, call_conf_slot);
+
+                /* Save state */
+                app_config.dyn_player_id = player_id;
+                app_config.dyn_player_port = player_port;
+                app_config.dyn_player_call = ci->id;
+
+                PJ_LOG(3, (THIS_FILE, "Dynamic playback auto-started"));
+                connect_sound = PJ_FALSE;
+            } else {
+                PJ_LOG(2, (THIS_FILE, "Failed to auto-start playback: %d", status));
+                app_config.dyn_player_active = PJ_FALSE;
+            }


This code for creating and connecting the player is duplicated from start_playback in pjsua_app_cli.c (lines 1446-1468). Consider extracting this logic into a shared helper function to reduce code duplication and improve maintainability.

pjsua_app.c (callback context) already has call_conf_slot and ci->id available from callback, has no intermediate error checking for conf_connect
pjsua_app_cli.c (CLI command) must find and validate call/media slots itself, error handling with cleanup at each step, returns status to caller
A shared helper would require passing many parameters (call_id, call_conf_slot, filename, etc.), handling different error processing.
Don't you think that it will add more complexity?

Copilot · 2025-12-30T06:56:21Z

pjsip-apps/src/pjsua/pjsua_app_cli.c

+    /* Connect microphone to recorder */
+    status = pjsua_conf_connect(0, rec_port);
+    if (status != PJ_SUCCESS) {
+        PJ_LOG(2, (THIS_FILE, "Failed to connect mic to recorder: %d", status));
+        /* Continue anyway, we at least have call audio */
+    }
+
+    /* Save state */
+    app_config.dyn_rec_id = rec_id;
+    app_config.dyn_rec_port = rec_port;
+    app_config.dyn_rec_call = call_id;
+
+    PJ_LOG(3, (THIS_FILE, "Recording started: %s", filename));
+    return PJ_SUCCESS;


When connecting the microphone to the recorder fails on line 1630, the function continues and saves the recorder state including rec_port and rec_id. However, only the call audio is connected, not the microphone. Later, in stop_recording (line 1683), the code unconditionally disconnects the microphone from rec_port, which may fail or be unnecessary since it was never successfully connected. Consider tracking whether the microphone was successfully connected to avoid unnecessary disconnect operations.

The disconnect of a never-connected port returns PJ_EINVAL but is harmless - no crash or resource leak. However, if you prefer the explicit tracking, I can add some flag.

nanangizz · 2026-01-05T03:10:54Z

Just an idea.

As the new command only affects a specific call (i.e: current or next call), perhaps the command should start with call instead of audio. For example:

call <call-id> playwav start <filename> - Start playing a WAV file to a call. If call-id is -1 play to the current active call, or the next call if no current active call.
call <call-id> recwav start <filename> - Start recording the call to a file. If call-id is -1 play to the current active call, or the next call if no current active call.
call <call-id> playwav/recwav stop

This will improve the feature to be able to accurately target a specific call. Also, in the future it may be improved further by allowing WAV file playback in multiple calls concurrently (dyn_player_id etc belongs to pjsua_call ~~instead of global var~~, the global var may still be needed for queuing a WAV file for the next call). This can be useful for developing an IVR app.

vasilevalex · 2026-01-06T09:43:11Z

I like the idea with specifying <call-id>. I'll try to do this.
I also need to add tests.
About the scenario with multiple call recordings, what should we do with the microphone?

nanangizz · 2026-01-07T09:10:35Z

About the scenario with multiple call recordings, what should we do with the microphone?

I think we should leave the mic and any other connections as they are?

Add start/stop commands to play/rec audio files

ba603d5

nanangizz requested review from Copilot, nanangizz, sauwming and trengginas December 30, 2025 06:51

nanangizz added type: enhancement component: applications labels Dec 30, 2025

nanangizz added this to the release-2.17 milestone Dec 30, 2025

Copilot started reviewing on behalf of nanangizz December 30, 2025 06:51 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

pjsua: Fix state cleanup in dynamic playback/recording error paths

65a77f4

-        stop_playback();
+        status = stop_playback();
+        if (status != PJ_SUCCESS) {
+            PJ_PERROR(1, (THIS_FILE, status,
+                          "Failed to stop existing playback"));
+            return status;
+        }

-        stop_recording();
+        status = stop_recording();
+        if (status != PJ_SUCCESS) {
+            PJ_PERROR(1,(THIS_FILE, status,
+                         "Error stopping current recording before starting "
+                         "dynamic recording"));
+            return status;
+        }

	app_config.dyn_rec_active = PJ_FALSE;
	app_config.dyn_rec_active = PJ_FALSE;
	app_config.dyn_rec_filename[0] = '\0';

	app_config.dyn_player_active = PJ_FALSE;
	app_config.dyn_player_active = PJ_FALSE;
	app_config.dyn_play_filename[0] = '\0';

pjsua: Add audio play/rec commands to CLI framework #4723

Are you sure you want to change the base?

pjsua: Add audio play/rec commands to CLI framework #4723

Conversation

vasilevalex commented Dec 29, 2025

Summary

New Commands

Features

Behavior with Multiple Calls

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

vasilevalex Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

vasilevalex Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

vasilevalex Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

vasilevalex Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

nanangizz commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vasilevalex commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nanangizz commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nanangizz commented Jan 5, 2026 •

edited

Loading

vasilevalex commented Jan 6, 2026 •

edited

Loading