<p>fixeria has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/gapk/+/16154">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">procqueue/ecu: use generic ECU abstraction API<br><br>Change-Id: I434a5973add38f92fc2ebe7f16125cfcb8ff9e23<br>---<br>M include/osmocom/gapk/codecs.h<br>M src/Makefile.am<br>M src/codec_fr.c<br>D src/ecu_fr.c<br>M src/pq_ecu.c<br>5 files changed, 80 insertions(+), 105 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/gapk refs/changes/54/16154/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/gapk/codecs.h b/include/osmocom/gapk/codecs.h</span><br><span>index 33f6593..253fb14 100644</span><br><span>--- a/include/osmocom/gapk/codecs.h</span><br><span>+++ b/include/osmocom/gapk/codecs.h</span><br><span>@@ -73,9 +73,6 @@</span><br><span> /* Encoding / decoding function pointers */</span><br><span> osmo_gapk_codec_conv_cb_t codec_encode;</span><br><span> osmo_gapk_codec_conv_cb_t codec_decode;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* ECU (Error Concealment Unit) function */</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_gapk_codec_conv_cb_t ecu_proc;</span><br><span> };</span><br><span> </span><br><span> const struct osmo_gapk_codec_desc *</span><br><span>diff --git a/src/Makefile.am b/src/Makefile.am</span><br><span>index 8fdeff7..94d9356 100644</span><br><span>--- a/src/Makefile.am</span><br><span>+++ b/src/Makefile.am</span><br><span>@@ -46,11 +46,6 @@</span><br><span> pq_ecu.c \</span><br><span> $(NULL)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-# ECU (Error Concealment Unit) implementation</span><br><span style="color: hsl(0, 100%, 40%);">-libosmogapk_la_SOURCES += \</span><br><span style="color: hsl(0, 100%, 40%);">- ecu_fr.c \</span><br><span style="color: hsl(0, 100%, 40%);">- $(NULL)</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> # Formats implementation</span><br><span> libosmogapk_la_SOURCES += \</span><br><span> formats.c \</span><br><span>diff --git a/src/codec_fr.c b/src/codec_fr.c</span><br><span>index 75efeb7..f2ca6e6 100644</span><br><span>--- a/src/codec_fr.c</span><br><span>+++ b/src/codec_fr.c</span><br><span>@@ -83,9 +83,6 @@</span><br><span> </span><br><span> #endif /* HAVE_LIBGSM */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Forward declaration of ECU (Error Concealment Unit) function */</span><br><span style="color: hsl(0, 100%, 40%);">-int ecu_proc_fr(void *state, uint8_t *out, const uint8_t *in, unsigned int in_len);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> const struct osmo_gapk_codec_desc codec_fr_desc = {</span><br><span> .type = CODEC_FR,</span><br><span> .name = "fr",</span><br><span>@@ -99,5 +96,4 @@</span><br><span> .codec_encode = codec_fr_encode,</span><br><span> .codec_decode = codec_fr_decode,</span><br><span> #endif</span><br><span style="color: hsl(0, 100%, 40%);">- .ecu_proc = ecu_proc_fr,</span><br><span> };</span><br><span>diff --git a/src/ecu_fr.c b/src/ecu_fr.c</span><br><span>deleted file mode 100644</span><br><span>index db55fec..0000000</span><br><span>--- a/src/ecu_fr.c</span><br><span>+++ /dev/null</span><br><span>@@ -1,78 +0,0 @@</span><br><span style="color: hsl(0, 100%, 40%);">-/* ECU (Error Concealment Unit) for FR */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-/*</span><br><span style="color: hsl(0, 100%, 40%);">- * This file is part of GAPK (GSM Audio Pocket Knife).</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * (C) 2018 by Vadim Yanitskiy <axilirator@gmail.com></span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * All Rights Reserved</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * GAPK is free software: you can redistribute it and/or modify</span><br><span style="color: hsl(0, 100%, 40%);">- * it under the terms of the GNU General Public License as published by</span><br><span style="color: hsl(0, 100%, 40%);">- * the Free Software Foundation, either version 3 of the License, or</span><br><span style="color: hsl(0, 100%, 40%);">- * (at your option) any later version.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * GAPK is distributed in the hope that it will be useful,</span><br><span style="color: hsl(0, 100%, 40%);">- * but WITHOUT ANY WARRANTY; without even the implied warranty of</span><br><span style="color: hsl(0, 100%, 40%);">- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the</span><br><span style="color: hsl(0, 100%, 40%);">- * GNU General Public License for more details.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * You should have received a copy of the GNU General Public License</span><br><span style="color: hsl(0, 100%, 40%);">- * along with GAPK. If not, see <http://www.gnu.org/licenses/>.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-#include <stdint.h></span><br><span style="color: hsl(0, 100%, 40%);">-#include <string.h></span><br><span style="color: hsl(0, 100%, 40%);">-#include <assert.h></span><br><span style="color: hsl(0, 100%, 40%);">-#include <stdbool.h></span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-#include <osmocom/gapk/codecs.h></span><br><span style="color: hsl(0, 100%, 40%);">-#include <osmocom/codec/ecu.h></span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-int</span><br><span style="color: hsl(0, 100%, 40%);">-ecu_proc_fr(void *_state, uint8_t *out, const uint8_t *in, unsigned int in_len)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- struct osmo_ecu_fr_state *state = (struct osmo_ecu_fr_state *) _state;</span><br><span style="color: hsl(0, 100%, 40%);">- bool backup = false;</span><br><span style="color: hsl(0, 100%, 40%);">- bool bfi = true;</span><br><span style="color: hsl(0, 100%, 40%);">- int i, rc;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- assert(in_len == FR_CANON_LEN);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Check if a frame is BFI */</span><br><span style="color: hsl(0, 100%, 40%);">- for (i = 1; i < in_len; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (in[i] != 0x00) {</span><br><span style="color: hsl(0, 100%, 40%);">- bfi = false;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* We have got a good frame, nothing to do */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!bfi) {</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_ecu_fr_reset(state, (uint8_t *) in);</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(out, in, in_len);</span><br><span style="color: hsl(0, 100%, 40%);">- return in_len;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Check if ECU state contains a backup frame */</span><br><span style="color: hsl(0, 100%, 40%);">- for (i = 0; i < in_len; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (state->frame_backup[i] != 0x00) {</span><br><span style="color: hsl(0, 100%, 40%);">- backup = true;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* There is no back-up frame */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!backup) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Copy BFI 'as-is' */</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(out, in, in_len);</span><br><span style="color: hsl(0, 100%, 40%);">- return in_len;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Attempt to perform error concealment */</span><br><span style="color: hsl(0, 100%, 40%);">- rc = osmo_ecu_fr_conceal(state, out);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc)</span><br><span style="color: hsl(0, 100%, 40%);">- return rc;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return in_len;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span>diff --git a/src/pq_ecu.c b/src/pq_ecu.c</span><br><span>index fad853c..a9972b1 100644</span><br><span>--- a/src/pq_ecu.c</span><br><span>+++ b/src/pq_ecu.c</span><br><span>@@ -3,7 +3,7 @@</span><br><span> /*</span><br><span> * This file is part of GAPK (GSM Audio Pocket Knife).</span><br><span> *</span><br><span style="color: hsl(0, 100%, 40%);">- * (C) 2018 by Vadim Yanitskiy <axilirator@gmail.com></span><br><span style="color: hsl(120, 100%, 40%);">+ * (C) 2018-2019 by Vadim Yanitskiy <axilirator@gmail.com></span><br><span> *</span><br><span> * All Rights Reserved</span><br><span> *</span><br><span>@@ -22,20 +22,69 @@</span><br><span> */</span><br><span> </span><br><span> #include <stdint.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <string.h></span><br><span> #include <talloc.h></span><br><span> #include <errno.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/codec/codec.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/codec/ecu.h></span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #include <osmocom/gapk/procqueue.h></span><br><span> #include <osmocom/gapk/logging.h></span><br><span> #include <osmocom/gapk/formats.h></span><br><span> #include <osmocom/gapk/codecs.h></span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#include <osmocom/codec/ecu.h></span><br><span style="color: hsl(120, 100%, 40%);">+/* Empty buffer for quick memcmp() based BFI detection */</span><br><span style="color: hsl(120, 100%, 40%);">+static const uint8_t zero_buf[40] = { 0x00 };</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static int</span><br><span style="color: hsl(120, 100%, 40%);">+pq_cb_ecu_proc(void *state, uint8_t *out, const uint8_t *in, unsigned int in_len)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_ecu_state *ecu_state = state;</span><br><span style="color: hsl(120, 100%, 40%);">+ int bfi, rc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Check if the input frame is BFI */</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (ecu_state->codec) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case OSMO_ECU_CODEC_HR:</span><br><span style="color: hsl(120, 100%, 40%);">+ bfi = !memcmp(zero_buf, in + 1, GSM_HR_BYTES);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case OSMO_ECU_CODEC_FR:</span><br><span style="color: hsl(120, 100%, 40%);">+ bfi = !memcmp(zero_buf, in + 1, GSM_FR_BYTES - 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case OSMO_ECU_CODEC_EFR:</span><br><span style="color: hsl(120, 100%, 40%);">+ bfi = !memcmp(zero_buf, in + 1, GSM_EFR_BYTES - 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TODO: implement BFI detection (in[0] == AMR_NO_DATA?) */</span><br><span style="color: hsl(120, 100%, 40%);">+ case OSMO_ECU_CODEC_AMR:</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPGAPK(LOGL_FATAL, "Unknown ECU codec\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(0);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_ecu_frame_in(state, bfi, in, in_len);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPGAPK(LOGL_ERROR, "osmo_ecu_frame_in() failed: rc=%d\n", rc);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EIO;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (bfi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_ecu_frame_out(state, out);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc >= 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPGAPK(LOGL_NOTICE, "osmo_ecu_frame_out() failed: rc=%d\n", rc);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Forward BFI 'as-is' */</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Forward frame 'as-is' */</span><br><span style="color: hsl(120, 100%, 40%);">+ memcpy(out, in, in_len);</span><br><span style="color: hsl(120, 100%, 40%);">+ return in_len;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span> </span><br><span> static void</span><br><span style="color: hsl(0, 100%, 40%);">-clean_up(void *state)</span><br><span style="color: hsl(120, 100%, 40%);">+pq_cb_ecu_exit(void *state)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- talloc_free(state);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_ecu_state *ecu_state = state;</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_ecu_destroy(ecu_state);</span><br><span> }</span><br><span> </span><br><span> /*! Add a ECU block to the processing queue</span><br><span>@@ -47,23 +96,37 @@</span><br><span> const struct osmo_gapk_codec_desc *codec)</span><br><span> {</span><br><span> const struct osmo_gapk_format_desc *fmt;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_ecu_state *ecu_state;</span><br><span> struct osmo_gapk_pq_item *item;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Allocate the ECU state */</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (codec->type) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case CODEC_HR:</span><br><span style="color: hsl(120, 100%, 40%);">+ ecu_state = osmo_ecu_init(pq, OSMO_ECU_CODEC_HR);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case CODEC_FR:</span><br><span style="color: hsl(120, 100%, 40%);">+ ecu_state = osmo_ecu_init(pq, OSMO_ECU_CODEC_FR);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case CODEC_EFR:</span><br><span style="color: hsl(120, 100%, 40%);">+ ecu_state = osmo_ecu_init(pq, OSMO_ECU_CODEC_EFR);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FIXME: we need to know how to detect AMR BFI */</span><br><span style="color: hsl(120, 100%, 40%);">+ case CODEC_AMR:</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ ecu_state = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Make sure that a codec has corresponding ECU implementation */</span><br><span style="color: hsl(0, 100%, 40%);">- if (codec->ecu_proc == NULL) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ecu_state == NULL) {</span><br><span> LOGPGAPK(LOGL_ERROR, "Codec '%s' has no ECU implementation\n", codec->name);</span><br><span> return -ENOTSUP;</span><br><span> }</span><br><span> </span><br><span> /* Allocate a new item to the processing queue */</span><br><span> item = osmo_gapk_pq_add_item(pq);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!item)</span><br><span style="color: hsl(0, 100%, 40%);">- return -ENOMEM;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Allocate the ECU state */</span><br><span style="color: hsl(0, 100%, 40%);">- item->state = talloc_zero(item, struct osmo_ecu_fr_state);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!item->state) {</span><br><span style="color: hsl(0, 100%, 40%);">- talloc_free(item);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!item) {</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_ecu_destroy(ecu_state);</span><br><span> return -ENOMEM;</span><br><span> }</span><br><span> </span><br><span>@@ -72,9 +135,11 @@</span><br><span> item->len_in = fmt->frame_len;</span><br><span> item->len_out = fmt->frame_len;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- item->proc = codec->ecu_proc;</span><br><span style="color: hsl(0, 100%, 40%);">- item->exit = &clean_up;</span><br><span style="color: hsl(0, 100%, 40%);">- item->wait = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ talloc_steal(item, ecu_state);</span><br><span style="color: hsl(120, 100%, 40%);">+ item->state = ecu_state;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ item->proc = &pq_cb_ecu_proc;</span><br><span style="color: hsl(120, 100%, 40%);">+ item->exit = &pq_cb_ecu_exit;</span><br><span> </span><br><span> /* Meta information */</span><br><span> item->type = OSMO_GAPK_ITEM_TYPE_PROC;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/gapk/+/16154">change 16154</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/gapk/+/16154"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: gapk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I434a5973add38f92fc2ebe7f16125cfcb8ff9e23 </div>
<div style="display:none"> Gerrit-Change-Number: 16154 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>