[PATCH 05/11] mgcp: Add RTP audio transcoding

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Jacob Erlbeck jerlbeck at sysmocom.de
Wed May 14 12:44:26 UTC 2014


Hello Holger,

I'm already modifying the patches and planning to send them tomorrow.

On 14.05.2014 07:46, Holger Hans Peter Freyther wrote:
> On Mon, May 12, 2014 at 12:39:01PM +0200, Jacob Erlbeck wrote:
> 
>> +#include "g711common.h"
>> +#include <gsm.h>
>> +#include <bcg729/decoder.h>
>> +#include <bcg729/encoder.h>
> 
> these must be guarded with the approriate defines. Specially the g729
> ones. I think you are lucky as /usr/include/bcg729/encoder.h just
> exists. :)

Thanks for spotting those, they're just left overs and can be removed
completely (the real ones are in mpgc_transcode.c are guarded).

> 
>> +++ b/openbsc/src/osmo-bsc_mgcp/mgcp_transcode.c
>> @@ -0,0 +1,459 @@
>> +/*
>> + * (C) 2014 by On-Waves
> 
> shared copyright here. :)

By whom?

> 
>> +#include "../../bscconfig.h"
> 
> Does this work with make distcheck? srcdir != builddir?

Apparently yes. This line dates back to 2010
(5a29c7fa895a14112a1ac65e541f4b87ae9dba04) and can be found like that at
several other places too. Removing the '../../' works too, but I don't
want to change that within this commit.

> 
>> +	/* cleanup first */
>> +	if (state) {
>> +		talloc_free(state);
>> +		dst_end->rtp_process_data = NULL;
> 
> 		state = NULL;
>> +	}
> 
> 
> Or just avoid assigning state that early?

This might called multiply during a single call and this way does a full
transcoding reset every time including cleaning up and disabling it when
called with src_end == NULL.

> 
>> +		LOGP(DMGCP, LOGL_ERROR,
>> +		     "Cannot transcode: %s codec not supported (%s -> %s).\n",
>> +		     src_fmt != AF_INVALID ? "destination" : "source",
>> +		     src_end->audio_name, dst_end->audio_name);
>> +		return -EINVAL;
> 
> Will the CRCX/MDCX fail in this case? I am a bit too lazy to check this
> right now.

No. It just falls back to passing all packets unmodified. I'd rather
like to address this when real negiation is added, but OTOH it wouldn't
be much effort to just terminate the connection in this case.

> 
>> +	/* TODO: remove me
>> +	fprintf(stderr, "sample_cnt = %d, sample_idx = %d, plen = %d -> %d, "
>> +		"hdr_size = %d, len = %d, pt = %d\n",
>> +	       sample_cnt, sample_idx, payload_len, nbytes, rtp_hdr_size, *len,
>> +	       data[1]);
>> +	       */
> 
> You want to keep this for now?

No.

> 
>> +#ifndef OPENBSC_MGCP_TRANSCODE_H
>> +#define OPENBSC_MGCP_TRANSCODE_H
> 
> I started to use "#pragma once". It is supported by GCC for a long
> time and even the Microsoft Compiler handles it correctly.
> 

Hmm, I just found that in only single file yet. I'm just traditionally
reluctant with #pragma'a in general, but if we want to change the style
I can change this accordingly.


-- 
- Jacob Erlbeck <jerlbeck at sysmocom.de>       http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte




More information about the OpenBSC mailing list