[pulseaudio-discuss,3/8] rtp: Drop support for non-L16 media

Submitted by Arun Raghavan on Feb. 29, 2016, 10:16 a.m.

Details

Message ID 1456740996-13908-4-git-send-email-arun@accosted.net
State New
Headers show
Series "Add GStreamer-based RTP support" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Arun Raghavan Feb. 29, 2016, 10:16 a.m.
From: Arun Raghavan <git@arunraghavan.net>

There doesn't seem much value in supporting streaming U8/mulaw/alaw on
the network, and it's unlikely these get any testing. Makes more sense
to drop these formats and just convert to L16 if we're dealing with
source media in that format.
---
 src/modules/rtp/rtp.c | 34 +---------------------------------
 src/modules/rtp/sdp.c |  9 ---------
 2 files changed, 1 insertion(+), 42 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/rtp/rtp.c b/src/modules/rtp/rtp.c
index dfc295f..a5a5082 100644
--- a/src/modules/rtp/rtp.c
+++ b/src/modules/rtp/rtp.c
@@ -320,10 +320,6 @@  fail:
 uint8_t pa_rtp_payload_from_sample_spec(const pa_sample_spec *ss) {
     pa_assert(ss);
 
-    if (ss->format == PA_SAMPLE_ULAW && ss->rate == 8000 && ss->channels == 1)
-        return 0;
-    if (ss->format == PA_SAMPLE_ALAW && ss->rate == 8000 && ss->channels == 1)
-        return 8;
     if (ss->format == PA_SAMPLE_S16BE && ss->rate == 44100 && ss->channels == 2)
         return 10;
     if (ss->format == PA_SAMPLE_S16BE && ss->rate == 44100 && ss->channels == 1)
@@ -336,18 +332,6 @@  pa_sample_spec *pa_rtp_sample_spec_from_payload(uint8_t payload, pa_sample_spec
     pa_assert(ss);
 
     switch (payload) {
-        case 0:
-            ss->channels = 1;
-            ss->format = PA_SAMPLE_ULAW;
-            ss->rate = 8000;
-            break;
-
-        case 8:
-            ss->channels = 1;
-            ss->format = PA_SAMPLE_ALAW;
-            ss->rate = 8000;
-            break;
-
         case 10:
             ss->channels = 2;
             ss->format = PA_SAMPLE_S16BE;
@@ -383,11 +367,7 @@  int pa_rtp_sample_spec_valid(const pa_sample_spec *ss) {
     if (!pa_sample_spec_valid(ss))
         return 0;
 
-    return
-        ss->format == PA_SAMPLE_U8 ||
-        ss->format == PA_SAMPLE_ALAW ||
-        ss->format == PA_SAMPLE_ULAW ||
-        ss->format == PA_SAMPLE_S16BE;
+    return ss->format == PA_SAMPLE_S16BE;
 }
 
 void pa_rtp_context_destroy(pa_rtp_context *c) {
@@ -403,12 +383,6 @@  const char* pa_rtp_format_to_string(pa_sample_format_t f) {
     switch (f) {
         case PA_SAMPLE_S16BE:
             return "L16";
-        case PA_SAMPLE_U8:
-            return "L8";
-        case PA_SAMPLE_ALAW:
-            return "PCMA";
-        case PA_SAMPLE_ULAW:
-            return "PCMU";
         default:
             return NULL;
     }
@@ -419,12 +393,6 @@  pa_sample_format_t pa_rtp_string_to_format(const char *s) {
 
     if (pa_streq(s, "L16"))
         return PA_SAMPLE_S16BE;
-    else if (pa_streq(s, "L8"))
-        return PA_SAMPLE_U8;
-    else if (pa_streq(s, "PCMA"))
-        return PA_SAMPLE_ALAW;
-    else if (pa_streq(s, "PCMU"))
-        return PA_SAMPLE_ULAW;
     else
         return PA_SAMPLE_INVALID;
 }
diff --git a/src/modules/rtp/sdp.c b/src/modules/rtp/sdp.c
index 14953cf..6a2e0c9 100644
--- a/src/modules/rtp/sdp.c
+++ b/src/modules/rtp/sdp.c
@@ -89,15 +89,6 @@  static pa_sample_spec *parse_sdp_sample_spec(pa_sample_spec *ss, char *c) {
     if (pa_startswith(c, "L16/")) {
         ss->format = PA_SAMPLE_S16BE;
         c += 4;
-    } else if (pa_startswith(c, "L8/")) {
-        ss->format = PA_SAMPLE_U8;
-        c += 3;
-    } else if (pa_startswith(c, "PCMA/")) {
-        ss->format = PA_SAMPLE_ALAW;
-        c += 5;
-    } else if (pa_startswith(c, "PCMU/")) {
-        ss->format = PA_SAMPLE_ULAW;
-        c += 5;
     } else
         return NULL;
 

Comments

On Mon, 2016-02-29 at 15:46 +0530, arun@accosted.net wrote:
> From: Arun Raghavan <git@arunraghavan.net>
> 
> There doesn't seem much value in supporting streaming U8/mulaw/alaw on
> the network, and it's unlikely these get any testing. Makes more sense
> to drop these formats and just convert to L16 if we're dealing with
> source media in that format.

It seems likely that the main motivation behind removing the support is
not mentioned in the commit message. Something to do with gstreamer?
On Mon, 2016-04-11 at 19:23 +0300, Tanu Kaskinen wrote:
> On Mon, 2016-02-29 at 15:46 +0530, arun@accosted.net wrote:
> > 
> > From: Arun Raghavan <git@arunraghavan.net>
> > 
> > There doesn't seem much value in supporting streaming U8/mulaw/alaw on
> > the network, and it's unlikely these get any testing. Makes more sense
> > to drop these formats and just convert to L16 if we're dealing with
> > source media in that format.
> It seems likely that the main motivation behind removing the support is
> not mentioned in the commit message. Something to do with gstreamer?

Not sure what you mean. These are not likely to be used or tested. I
didn't want to add a bunch of equivalent kludge in the GStreamer
backend as well to support the same thing.

-- Arun
On Tue, 2016-04-12 at 09:56 +0530, Arun Raghavan wrote:
> On Mon, 2016-04-11 at 19:23 +0300, Tanu Kaskinen wrote:
> > On Mon, 2016-02-29 at 15:46 +0530, arun@accosted.net wrote:
> > > From: Arun Raghavan <git@arunraghavan.net>
> > > 
> > > There doesn't seem much value in supporting streaming U8/mulaw/alaw on
> > > the network, and it's unlikely these get any testing. Makes more sense
> > > to drop these formats and just convert to L16 if we're dealing with
> > > source media in that format.
> > 
> > It seems likely that the main motivation behind removing the support is
> > not mentioned in the commit message. Something to do with gstreamer?

> Not sure what you mean. These are not likely to be used or tested. I
> didn't want to add a bunch of equivalent kludge in the GStreamer
> backend as well to support the same thing.

That's the context I was looking for. You could add a note about the
relationship with the gstreamer code to the commit message, although
it's fine as it is too.

The code changes look good.

-- 
Tanu