Skip to content

Conversation

Fr-Soltanzadeh
Copy link

Hi,

This PR adds the capability to play audio directly from raw RTP payload file to improve performance .

New Command Parameters
The existing play_media NG command is extended with two new parameters:

audio-raw_rtp_file: Path to the file containing raw RTP payloads (no headers).

audio-raw_rtp_codec: Codec of the payloads (e.g., PCMU, PCMA).

Example Usage:

bash
curl -X POST http://localhost:2225/ng-plain -H "Content-Type: application/json" -d '{
"command": "play media",
"audio-raw-rtp-file": "/path/to/file.raw",
"audio-raw-rtp-codec":"PCMU",
"call-id": "123abc",
"from-tag": "456def"
}'

Codec Support: Initially supports G.711 PCMU and PCMA

All existing functionality remains unchanged. The implementation includes proper error handling and memory management.

Copy link
Member

@rfuchs rfuchs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. A few comments for you to consider. But I can merge this as is if you prefer.

I do wonder though if this couldn't have been achieved through some other means, for example by telling ffmpeg (libav...) that it is in fact opening a raw file. It does support both raw µ and A law input files, and for codecs that require framing (Opus...) it will need some more sophisticated processing anyway.

@@ -1104,6 +1104,57 @@ static int media_player_find_file_begin(struct media_player *mp) {
static bool media_player_read_packet(struct media_player *mp) {
if (!mp->coder.fmtctx)
return true;
// Handle raw RTP file playback
if (mp->coder.audio_raw_rtp_mode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since media_player_read_packet is already used as run_func function pointer, this whole block would lend itself to also be its own function and then be used as run_func, to avoid the if/else

// Set codec parameters based on input
if (opts.audio_raw_rtp_codec.len == 4 && strncasecmp(opts.audio_raw_rtp_codec.s, "PCMU", 4) == 0) {
mp->coder.audio_raw_rtp_codec = AV_CODEC_ID_PCM_MULAW;
mp->coder.audio_raw_rtp_frame_size = 160; // 20ms frames
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this dependent on the ptime?

long file_size = ftell(f);
fseek(f, 0, SEEK_SET);
// Read entire file into memory
char *file_data = malloc(file_size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tend to use g_malloc and g_free in the code.

Also glib provides a convenient g_file_get_contents() helper to do just this.


// Simulate packet timing (20ms per frame)
mp->coder.pkt->pts = mp->last_frame_ts;
mp->coder.pkt->duration = 160;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably depend on either ptime or the frame size variable?

fclose(f);

// Store in player context
mp->coder.audio_raw_rtp_data.s = file_data;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think this is never freed? So that definitely needs to be fixed

@Fr-Soltanzadeh Fr-Soltanzadeh force-pushed the Feature/1-play-audio-raw-rtp-file branch from 7d80f05 to 31e3c03 Compare September 7, 2025 12:00
@Fr-Soltanzadeh
Copy link
Author

Fr-Soltanzadeh commented Sep 7, 2025

Thank you for the feedback. As you mentioned, I used ffmpeg functions to open raw rtp file.

@rfuchs
Copy link
Member

rfuchs commented Sep 8, 2025

Very nice, thank you 👍

Question though, doesn't this now still go through the read -> decode -> encode -> send process? If the purpose is to improve performance then I suppose the decode/encode steps should be skipped and it should go directly from read to send?

media_player_play_start calls __ensure_codec_handler which calls __media_player_setup_internal which calls codec_handler_make_media_player to create the handler, which through its handler_func is in charge of processing audio packets, which I'm pretty sure is set up as a transcode handler and so would do decode+encode.

Probably something similar to what media_player_cached_reader_start does might make more sense? It might be enough to make sure that the handler is set up not as a transcode handler but as a passthrough handler.

Just a hint anyway, I can merge it as it is if you prefer.

Please do add the new options to the documentation though 😁

@Fr-Soltanzadeh
Copy link
Author

Thank you for the feedback. You're absolutely right - the current implementation still goes through decode/encode, which defeats the performance purpose. I'll revise it to use a passthrough handler instead.

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.

2 participants