[pulseaudio-commits] [Git][pulseaudio/pulseaudio][master] 5 commits: bluetooth/gst: Add assertions around element_add_pad and bin_add

PulseAudio Marge Bot gitlab at gitlab.freedesktop.org
Fri Jan 22 14:35:26 UTC 2021



PulseAudio Marge Bot pushed to branch master at PulseAudio / pulseaudio


Commits:
db73004a by Marijn Suijten at 2021-01-22T09:47:09+01:00
bluetooth/gst: Add assertions around element_add_pad and bin_add

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/484>

- - - - -
da1600eb by Marijn Suijten at 2021-01-22T10:41:19+01:00
bluetooth/gst: Determine PA input/output caps in generic code

Make the code ever so slightly more generic by not using appsrc and
appsink in codec-specific logic when assigning caps specific to the raw
(PCM) format provided by or returned to PA.

Note that caps have to be set (= event) after starting, can't send
events in flushing state.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/484>

- - - - -
73c80ffb by Marijn Suijten at 2021-01-22T10:41:20+01:00
bluetooth/aptx: Deduplicate caps setup for encoding and decoding

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/484>

- - - - -
46a97d76 by Marijn Suijten at 2021-01-22T10:41:20+01:00
bluetooth/aptx: Use capsfilter instead of appsink/appsrc "caps" prop

Make the codec-specific initializer more generic by not touching any
elements outside of its own GstBin.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/484>

- - - - -
9431e96a by Marijn Suijten at 2021-01-22T11:04:19+01:00
bluetooth/gst: Move common enc/dec initialization back to generic init

Now that codec-specific code only touches its own bin and not any
elements (appsink/src) outside of it, make things official by
initializng them later in gst_codec_init where they are actually needed.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/484>

- - - - -


4 changed files:

- src/modules/bluetooth/a2dp-codec-aptx-gst.c
- src/modules/bluetooth/a2dp-codec-gst.c
- src/modules/bluetooth/a2dp-codec-gst.h
- src/modules/bluetooth/a2dp-codec-ldac-gst.c


Changes:

=====================================
src/modules/bluetooth/a2dp-codec-aptx-gst.c
=====================================
@@ -269,8 +269,8 @@ static uint8_t fill_preferred_configuration_hd(const pa_sample_spec *default_sam
 }
 
 bool gst_init_aptx(struct gst_info *info, pa_sample_spec *ss, bool for_encoding) {
-    GstElement *enc, *dec;
-    GstCaps *caps;
+    GstElement *enc, *dec, *capsf;
+    GstCaps *caps = NULL;
     GstPad *pad;
     const char *aptx_codec_media_type;
 
@@ -334,84 +334,73 @@ bool gst_init_aptx(struct gst_info *info, pa_sample_spec *ss, bool for_encoding)
 
     aptx_codec_media_type = info->codec_type == APTX_HD ? "audio/aptx-hd" : "audio/aptx";
 
+    caps = gst_caps_new_simple(aptx_codec_media_type,
+            "rate", G_TYPE_INT, (int) ss->rate,
+            "channels", G_TYPE_INT, (int) ss->channels,
+            NULL);
+
+    capsf = gst_element_factory_make("capsfilter", "aptx_capsfilter");
+    if (!capsf) {
+        pa_log_error("Could not create aptX capsfilter element");
+        goto fail;
+    }
+    g_object_set(capsf, "caps", caps, NULL);
+
     if (for_encoding) {
         enc = gst_element_factory_make("openaptxenc", "aptx_encoder");
 
         if (enc == NULL) {
             pa_log_error("Could not create aptX encoder element");
-            goto fail;
+            goto fail_enc_dec;
         }
 
-        caps = gst_caps_new_simple("audio/x-raw",
-                "format", G_TYPE_STRING, "S24LE",
-                "rate", G_TYPE_INT, (int) ss->rate,
-                "channels", G_TYPE_INT, (int) ss->channels,
-                "channel-mask", G_TYPE_INT, 0,
-                "layout", G_TYPE_STRING, "interleaved",
-                NULL);
-        g_object_set(info->enc_src, "caps", caps, NULL);
-        gst_caps_unref(caps);
-
-        caps = gst_caps_new_simple(aptx_codec_media_type,
-                "rate", G_TYPE_INT, (int) ss->rate,
-                "channels", G_TYPE_INT, (int) ss->channels,
-                NULL);
-        g_object_set(info->enc_sink, "caps", caps, NULL);
-        gst_caps_unref(caps);
-
         info->enc_bin = gst_bin_new("aptx_enc_bin");
         pa_assert(info->enc_bin);
 
-        gst_bin_add(GST_BIN(info->enc_bin), enc);
+        gst_bin_add_many(GST_BIN(info->enc_bin), enc, capsf, NULL);
+        pa_assert_se(gst_element_link_many(enc, capsf, NULL));
 
         pad = gst_element_get_static_pad(enc, "sink");
-        gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("sink", pad));
+        pa_assert_se(gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("sink", pad)));
         gst_object_unref(GST_OBJECT(pad));
 
-        pad = gst_element_get_static_pad(enc, "src");
-        gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("src", pad));
+        pad = gst_element_get_static_pad(capsf, "src");
+        pa_assert_se(gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("src", pad)));
         gst_object_unref(GST_OBJECT(pad));
     } else {
         dec = gst_element_factory_make("openaptxdec", "aptx_decoder");
 
         if (dec == NULL) {
             pa_log_error("Could not create aptX decoder element");
-            goto fail;
+            goto fail_enc_dec;
         }
 
-        caps = gst_caps_new_simple(aptx_codec_media_type,
-                "rate", G_TYPE_INT, (int) ss->rate,
-                "channels", G_TYPE_INT, (int) ss->channels,
-                NULL);
-        g_object_set(info->dec_src, "caps", caps, NULL);
-        gst_caps_unref(caps);
-
-        caps = gst_caps_new_simple("audio/x-raw",
-                "format", G_TYPE_STRING, "S24LE",
-                "rate", G_TYPE_INT, (int) ss->rate,
-                "channels", G_TYPE_INT, (int) ss->channels,
-                "layout", G_TYPE_STRING, "interleaved",
-                NULL);
-        g_object_set(info->dec_sink, "caps", caps, NULL);
-        gst_caps_unref(caps);
-
         info->dec_bin = gst_bin_new("aptx_dec_bin");
         pa_assert(info->dec_bin);
 
-        gst_bin_add(GST_BIN(info->dec_bin), dec);
+        gst_bin_add_many(GST_BIN(info->dec_bin), capsf, dec, NULL);
+        pa_assert_se(gst_element_link_many(capsf, dec, NULL));
 
-        pad = gst_element_get_static_pad(dec, "sink");
-        gst_element_add_pad(info->dec_bin, gst_ghost_pad_new("sink", pad));
+        pad = gst_element_get_static_pad(capsf, "sink");
+        pa_assert_se(gst_element_add_pad(info->dec_bin, gst_ghost_pad_new("sink", pad)));
         gst_object_unref(GST_OBJECT(pad));
 
         pad = gst_element_get_static_pad(dec, "src");
-        gst_element_add_pad(info->dec_bin, gst_ghost_pad_new("src", pad));
+        pa_assert_se(gst_element_add_pad(info->dec_bin, gst_ghost_pad_new("src", pad)));
         gst_object_unref(GST_OBJECT(pad));
     }
 
+    gst_caps_unref(caps);
+
     return true;
 
+fail_enc_dec:
+    gst_object_unref(GST_OBJECT(capsf));
+
 fail:
+    if (caps)
+        gst_caps_unref(caps);
+
     pa_log_error("aptX initialisation failed");
     return false;
 }
@@ -436,33 +425,14 @@ static void *init_common(enum a2dp_codec_type codec_type, bool for_encoding, boo
     } else
         pa_assert_not_reached();
 
-    /*
-     * The common encoder/decoder initialisation functions need to be called
-     * before the codec specific ones, as the codec specific initialisation
-     * function needs to set the caps specific property appropriately on the
-     * appsrc and appsink as per the sample spec and the codec.
-     */
-    if (for_encoding) {
-        if (!gst_init_enc_common(info))
-            goto fail;
-    } else {
-        if (!gst_init_dec_common(info))
-            goto fail;
-    }
-
     if (!gst_init_aptx(info, sample_spec, for_encoding))
-        goto enc_dec_fail;
+        goto fail;
 
     if (!gst_codec_init(info, for_encoding))
-        goto enc_dec_fail;
+        goto fail;
 
     return info;
 
-enc_dec_fail:
-    if (for_encoding)
-        gst_deinit_enc_common(info);
-    else
-        gst_deinit_dec_common(info);
 fail:
     if (info)
         pa_xfree(info);


=====================================
src/modules/bluetooth/a2dp-codec-gst.c
=====================================
@@ -82,7 +82,7 @@ static GstFlowReturn dec_sink_new_sample(GstAppSink *appsink, gpointer userdata)
     return GST_FLOW_OK;
 }
 
-void gst_deinit_enc_common(struct gst_info *info) {
+static void gst_deinit_enc_common(struct gst_info *info) {
     if (!info)
         return;
     if (info->enc_fdsem)
@@ -97,7 +97,7 @@ void gst_deinit_enc_common(struct gst_info *info) {
         gst_object_unref(info->enc_pipeline);
 }
 
-void gst_deinit_dec_common(struct gst_info *info) {
+static void gst_deinit_dec_common(struct gst_info *info) {
     if (!info)
         return;
     if (info->dec_fdsem)
@@ -306,13 +306,73 @@ static GstPadProbeReturn gst_decoder_buffer_probe(GstPad *pad, GstPadProbeInfo *
     return GST_PAD_PROBE_OK;
 }
 
+static GstCaps *gst_create_caps_from_sample_spec(const pa_sample_spec *ss) {
+    gchar *sample_format;
+    GstCaps *caps;
+    int channel_mask;
+
+    switch (ss->format) {
+        case PA_SAMPLE_S16LE:
+            sample_format = "S16LE";
+            break;
+        case PA_SAMPLE_S24LE:
+            sample_format = "S24LE";
+            break;
+        case PA_SAMPLE_S32LE:
+            sample_format = "S32LE";
+            break;
+        default:
+            pa_assert_not_reached();
+            break;
+    }
+
+    switch (ss->channels) {
+        case 1:
+            channel_mask = 0x1;
+            break;
+        case 2:
+            channel_mask = 0x3;
+            break;
+        default:
+            pa_assert_not_reached();
+            break;
+    }
+
+    caps = gst_caps_new_simple("audio/x-raw",
+            "format", G_TYPE_STRING, sample_format,
+            "rate", G_TYPE_INT, (int) ss->rate,
+            "channels", G_TYPE_INT, (int) ss->channels,
+            "channel-mask", GST_TYPE_BITMASK, channel_mask,
+            "layout", G_TYPE_STRING, "interleaved",
+            NULL);
+
+    pa_assert(caps);
+    return caps;
+}
+
 bool gst_codec_init(struct gst_info *info, bool for_encoding) {
     GstPad *pad;
+    GstCaps *caps;
 
     info->seq_num = 0;
 
+    if (for_encoding) {
+        if (!gst_init_enc_common(info))
+            goto fail_common;
+    } else {
+        if (!gst_init_dec_common(info))
+            goto fail_common;
+    }
+
+    caps = gst_create_caps_from_sample_spec(info->ss);
+
     /* In case if we ever have a codec which supports decoding but not encoding */
-    if (for_encoding && info->enc_bin) {
+    if (for_encoding) {
+        pa_assert(info->enc_bin);
+
+        g_object_set(info->enc_src, "caps", caps, NULL);
+        gst_caps_unref(caps);
+
         gst_bin_add_many(GST_BIN(info->enc_pipeline), info->enc_src, info->enc_bin, info->enc_sink, NULL);
 
         if (!gst_element_link_many(info->enc_src, info->enc_bin, info->enc_sink, NULL)) {
@@ -329,7 +389,12 @@ bool gst_codec_init(struct gst_info *info, bool for_encoding) {
         pad = gst_element_get_static_pad(info->enc_bin, "sink");
         gst_pad_add_probe(pad, GST_PAD_PROBE_TYPE_BUFFER, gst_encoder_buffer_probe, info, NULL);
         gst_object_unref(pad);
-    } else if (!for_encoding && info->dec_bin) {
+    } else {
+        pa_assert(info->dec_bin);
+
+        g_object_set(info->dec_sink, "caps", caps, NULL);
+        gst_caps_unref(caps);
+
         gst_bin_add_many(GST_BIN(info->dec_pipeline), info->dec_src, info->dec_bin, info->dec_sink, NULL);
 
         if (!gst_element_link_many(info->dec_src, info->dec_bin, info->dec_sink, NULL)) {
@@ -346,10 +411,9 @@ bool gst_codec_init(struct gst_info *info, bool for_encoding) {
         pad = gst_element_get_static_pad(info->dec_bin, "sink");
         gst_pad_add_probe(pad, GST_PAD_PROBE_TYPE_BUFFER, gst_decoder_buffer_probe, info, NULL);
         gst_object_unref(pad);
-    } else
-        pa_assert_not_reached();
+    }
 
-    pa_log_info("Gstreamer pipeline initialisation succeeded");
+    pa_log_info("GStreamer pipeline initialisation succeeded");
 
     return true;
 
@@ -359,7 +423,21 @@ enc_dec_fail:
     else
         gst_deinit_dec_common(info);
 
-    pa_log_error("Gstreamer pipeline initialisation failed");
+    pa_log_error("GStreamer pipeline initialisation failed");
+
+    return false;
+
+fail_common:
+    /* If common initialization fails these have not had their ownership
+     * transferred to the pipeline yet.
+     */
+    if (for_encoding)
+        gst_object_unref(info->enc_bin);
+    else
+        gst_object_unref(info->dec_bin);
+
+
+    pa_log_error("GStreamer pipeline creation failed");
 
     return false;
 }


=====================================
src/modules/bluetooth/a2dp-codec-gst.h
=====================================
@@ -59,8 +59,3 @@ bool gst_codec_init(struct gst_info *info, bool for_encoding);
 size_t gst_encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed);
 size_t gst_decode_buffer(void *codec_info, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed);
 void gst_codec_deinit(void *codec_info);
-
-bool gst_init_enc_common(struct gst_info *info);
-bool gst_init_dec_common(struct gst_info *info);
-void gst_deinit_enc_common(struct gst_info *info);
-void gst_deinit_dec_common(struct gst_info *info);


=====================================
src/modules/bluetooth/a2dp-codec-ldac-gst.c
=====================================
@@ -201,7 +201,6 @@ static uint8_t fill_preferred_configuration(const pa_sample_spec *default_sample
 bool gst_init_ldac(struct gst_info *info, pa_sample_spec *ss, bool for_encoding) {
     GstElement *rtpldacpay;
     GstElement *enc;
-    GstCaps *caps;
     GstPad *pad;
 
     if (!for_encoding) {
@@ -264,16 +263,6 @@ bool gst_init_ldac(struct gst_info *info, pa_sample_spec *ss, bool for_encoding)
             goto fail;
     }
 
-    caps = gst_caps_new_simple("audio/x-raw",
-            "format", G_TYPE_STRING, "S32LE",
-            "rate", G_TYPE_INT, (int) ss->rate,
-            "channels", G_TYPE_INT, (int) ss->channels,
-            "channel-mask", G_TYPE_INT, 0,
-            "layout", G_TYPE_STRING, "interleaved",
-            NULL);
-    g_object_set(info->enc_src, "caps", caps, NULL);
-    gst_caps_unref(caps);
-
     rtpldacpay = gst_element_factory_make("rtpldacpay", "rtp_ldac_pay");
     if (!rtpldacpay) {
         pa_log_error("Could not create RTP LDAC payloader element");
@@ -291,11 +280,11 @@ bool gst_init_ldac(struct gst_info *info, pa_sample_spec *ss, bool for_encoding)
     }
 
     pad = gst_element_get_static_pad(enc, "sink");
-    gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("sink", pad));
+    pa_assert_se(gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("sink", pad)));
     gst_object_unref(GST_OBJECT(pad));
 
     pad = gst_element_get_static_pad(rtpldacpay, "src");
-    gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("src", pad));
+    pa_assert_se(gst_element_add_pad(info->enc_bin, gst_ghost_pad_new("src", pad)));
     gst_object_unref(GST_OBJECT(pad));
 
     return true;
@@ -308,6 +297,11 @@ fail:
 static void *init_common(enum a2dp_codec_type codec_type, bool for_encoding, bool for_backchannel, const uint8_t *config_buffer, uint8_t config_size, pa_sample_spec *sample_spec, pa_core *core) {
     struct gst_info *info = NULL;
 
+    if (!for_encoding) {
+        pa_log_error("LDAC decoder not supported");
+        return NULL;
+    }
+
     info = pa_xnew0(struct gst_info, 1);
     pa_assert(info);
 
@@ -318,25 +312,14 @@ static void *init_common(enum a2dp_codec_type codec_type, bool for_encoding, boo
     info->a2dp_codec_t.ldac_config = (const a2dp_ldac_t *) config_buffer;
     pa_assert(config_size == sizeof(*(info->a2dp_codec_t.ldac_config)));
 
-    /*
-     * The common encoder/decoder initialisation functions need to be called
-     * before the codec specific ones, as the codec specific initialisation
-     * function needs to set the caps specific property appropriately on the
-     * appsrc and appsink as per the sample spec and the codec.
-     */
-    if (!gst_init_enc_common(info))
-        goto fail;
-
     if (!gst_init_ldac(info, sample_spec, for_encoding))
-        goto enc_fail;
+        goto fail;
 
     if (!gst_codec_init(info, for_encoding))
-        goto enc_fail;
+        goto fail;
 
     return info;
 
-enc_fail:
-    gst_deinit_enc_common(info);
 fail:
     if (info)
         pa_xfree(info);



View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/ca3fd62c712c682f0767731c1689618d074c9e54...9431e96ae498c3935fb3a4df29c1b88b4037eb3b

-- 
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/ca3fd62c712c682f0767731c1689618d074c9e54...9431e96ae498c3935fb3a4df29c1b88b4037eb3b
You're receiving this email because of your account on gitlab.freedesktop.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-commits/attachments/20210122/a087a975/attachment-0001.htm>


More information about the pulseaudio-commits mailing list