From ee01f1585287711de3459a14be7ef7a61fc96126 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 3 Jan 2019 10:57:57 -0500 Subject: [PATCH] fix non-default encoder bitrate not being applied Bitrates specified in codec-transcode-... options were applied to the decoder created during the offer. The matching encoder only gets created during the answer phase, at which point the specified bitrate must be copied from the decoder. fixes #681 Change-Id: Idc6a16a4493908d78bb0b48ae590aba046152af0 --- daemon/codec.c | 19 +++++++-- t/transcode-test.c | 104 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/daemon/codec.c b/daemon/codec.c index b8a169fca..5e558b132 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -59,6 +59,7 @@ struct codec_ssrc_handler { decoder_t *decoder; encoder_t *encoder; format_t encoder_format; + int bitrate; int ptime; int bytes_per_packet; unsigned long ts_in; // for DTMF dupe detection @@ -419,7 +420,16 @@ unsupported: // the sink does not support this codec -> transcode ilog(LOG_DEBUG, "Sink does not support codec " STR_FORMAT, STR_FMT(&pt->encoding_with_params)); dest_pt = pref_dest_codec; -transcode: +transcode:; + // look up the reverse side of this payload type, which is the decoder to our + // encoder. if any codec options such as bitrate were set during an offer, + // they're in the decoder // PT. copy them to the encoder PT. + struct rtp_payload_type *reverse_pt = g_hash_table_lookup(sink->codecs_recv, + &dest_pt->payload_type); + if (reverse_pt) { + if (!dest_pt->bitrate) + dest_pt->bitrate = reverse_pt->bitrate; + } MEDIA_SET(receiver, TRANSCODE); __make_transcoder(handler, pt, dest_pt); @@ -856,6 +866,7 @@ static struct ssrc_entry *__ssrc_handler_transcode_new(void *p) { ch->ts_out = random(); ch->ptime = h->dest_pt.ptime; ch->sample_buffer = g_string_new(""); + ch->bitrate = h->dest_pt.bitrate ? : h->dest_pt.codec_def->default_bitrate; format_t enc_format = { .clockrate = h->dest_pt.clock_rate * h->dest_pt.codec_def->clockrate_mult, @@ -866,7 +877,7 @@ static struct ssrc_entry *__ssrc_handler_transcode_new(void *p) { if (!ch->encoder) goto err; if (encoder_config_fmtp(ch->encoder, h->dest_pt.codec_def, - h->dest_pt.bitrate ? : h->dest_pt.codec_def->default_bitrate, + ch->bitrate, ch->ptime, &enc_format, &ch->encoder_format, &h->dest_pt.format_parameters)) goto err; @@ -880,10 +891,10 @@ static struct ssrc_entry *__ssrc_handler_transcode_new(void *p) { * h->dest_pt.codec_def->bits_per_sample / 8; ilog(LOG_DEBUG, "Encoder created with clockrate %i, %i channels, using sample format %i " - "(ptime %i for %i samples per frame and %i samples (%i bytes) per packet)", + "(ptime %i for %i samples per frame and %i samples (%i bytes) per packet, bitrate %i)", ch->encoder_format.clockrate, ch->encoder_format.channels, ch->encoder_format.format, ch->ptime, ch->encoder->samples_per_frame, ch->encoder->samples_per_packet, - ch->bytes_per_packet); + ch->bytes_per_packet, ch->bitrate); return &ch->h; diff --git a/t/transcode-test.c b/t/transcode-test.c index 6ac72ed27..4d73b42ef 100644 --- a/t/transcode-test.c +++ b/t/transcode-test.c @@ -14,7 +14,8 @@ GString *dtmf_logs; static str *sdup(char *s) { str *r = g_slice_alloc(sizeof(*r)); - str_init(r, s); + char *d = strdup(s); + str_init(r, d); return r; } static void queue_dump(GString *s, GQueue *q) { @@ -114,6 +115,24 @@ static void __expect(const char *file, int line, GQueue *dumper, const char *cod g_string_free(s, TRUE); } +#define check_encoder(side, in_pt, out_pt, out_bitrate) \ + __check_encoder(__FILE__, __LINE__, media_ ## side, in_pt, out_pt, out_bitrate) + +static void __check_encoder(const char *file, int line, struct call_media *m, int in_pt, int out_pt, + int out_bitrate) +{ + struct codec_handler *ch = g_hash_table_lookup(m->codec_handlers, &in_pt); + printf("running test %s:%i\n", file, line); + assert(ch->source_pt.payload_type == in_pt); + if (ch->dest_pt.payload_type != out_pt || ch->dest_pt.bitrate != out_bitrate) { + printf("test failed: %s:%i\n", file, line); + printf("expected: %i/%i\n", out_pt, out_bitrate); + printf("received: %i/%i\n", ch->dest_pt.payload_type, ch->dest_pt.bitrate); + abort(); + } + printf("test ok: %s:%i\n", file, line); +} + #define packet_seq_ts(side, pt_in, pload, rtp_ts, rtp_seq, pt_out, pload_exp, ts_exp, fatal) \ __packet_seq_ts( __FILE__, __LINE__, media_ ## side, pt_in, (str) STR_CONST_INIT(pload), \ (str) STR_CONST_INIT(pload_exp), ssrc_ ## side, rtp_ts, rtp_seq, pt_out, \ @@ -523,6 +542,89 @@ int main() { } } + { + str codec_name = STR_CONST_INIT("AMR"); + const codec_def_t *def = codec_find(&codec_name, MT_AUDIO); + assert(def); + if (def->support_encoding && def->support_decoding) { + // default bitrate + start(); + sdp_pt(0, PCMU, 8000); + transcode(AMR); + offer(); + expect(A, recv, ""); + expect(A, send, "0/PCMU/8000"); + expect(B, recv, "0/PCMU/8000 96/AMR/8000/octet-align=1"); + expect(B, send, ""); + sdp_pt_fmt(96, AMR, 8000, "octet-align=1"); + answer(); + expect(A, recv, "0/PCMU/8000"); + expect(A, send, "0/PCMU/8000"); + expect(B, recv, "96/AMR/8000/octet-align=1"); + expect(B, send, "96/AMR/8000/octet-align=1"); + check_encoder(A, 0, 96, 0); // uses codec default + check_encoder(B, 96, 0, 0); + end(); + + // default bitrate reverse + start(); + sdp_pt(96, AMR, 8000); + transcode(PCMU); + offer(); + expect(A, recv, ""); + expect(A, send, "96/AMR/8000"); + expect(B, recv, "96/AMR/8000 0/PCMU/8000"); + expect(B, send, ""); + sdp_pt(0, PCMU, 8000); + answer(); + expect(A, recv, "96/AMR/8000"); + expect(A, send, "96/AMR/8000"); + expect(B, recv, "0/PCMU/8000"); + expect(B, send, "0/PCMU/8000"); + check_encoder(A, 96, 0, 0); + check_encoder(B, 0, 96, 0); // uses codec default + end(); + + // specify forward bitrate + start(); + sdp_pt(0, PCMU, 8000); + transcode(AMR/8000/1/6700); + offer(); + expect(A, recv, ""); + expect(A, send, "0/PCMU/8000"); + expect(B, recv, "0/PCMU/8000 96/AMR/8000/octet-align=1"); + expect(B, send, ""); + sdp_pt_fmt(96, AMR, 8000, "octet-align=1"); + answer(); + expect(A, recv, "0/PCMU/8000"); + expect(A, send, "0/PCMU/8000"); + expect(B, recv, "96/AMR/8000/octet-align=1"); + expect(B, send, "96/AMR/8000/octet-align=1"); + check_encoder(A, 0, 96, 6700); + check_encoder(B, 96, 0, 0); + end(); + + // specify non-default forward bitrate + start(); + sdp_pt(0, PCMU, 8000); + transcode(AMR/8000/1/7400); + offer(); + expect(A, recv, ""); + expect(A, send, "0/PCMU/8000"); + expect(B, recv, "0/PCMU/8000 96/AMR/8000/octet-align=1"); + expect(B, send, ""); + sdp_pt_fmt(96, AMR, 8000, "octet-align=1"); + answer(); + expect(A, recv, "0/PCMU/8000"); + expect(A, send, "0/PCMU/8000"); + expect(B, recv, "96/AMR/8000/octet-align=1"); + expect(B, send, "96/AMR/8000/octet-align=1"); + check_encoder(A, 0, 96, 7400); + check_encoder(B, 96, 0, 0); + end(); + } + } + // G.722 <> PCMA start(); sdp_pt(8, PCMA, 8000);