Changeset 210584 in webkit


Ignore:
Timestamp:
Jan 11, 2017 3:27:38 AM (7 years ago)
Author:
Carlos Garcia Campos
Message:

[GStreamer] Use smart pointers and modernize code in WebKitWebAudioSourceGStreamer
​https://bugs.webkit.org/show_bug.cgi?id=166886

Reviewed by Xabier Rodriguez-Calvar.

This patch doesn't change the behavior, so it's covered by existing Web Audio tests. It replaces pointers with
smart pointers, uses WTF::Vector instead of GSList and simplifies the code to map/unmap GstBuffers.

  • platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:

(webKitWebAudioSrcConstructed):
(webKitWebAudioSrcFinalize):
(webKitWebAudioSrcLoop):
(webKitWebAudioSrcChangeState):

  • platform/graphics/gstreamer/GRefPtrGStreamer.cpp:

(WTF::derefGPtr<GstBufferList>):
(WTF::adoptGRef):
(WTF::refGPtr<GstBufferPool>):
(WTF::derefGPtr<GstBufferPool>):

  • platform/graphics/gstreamer/GRefPtrGStreamer.h:
  • platform/graphics/gstreamer/GStreamerUtilities.cpp:

(WebCore::mapGstBuffer):

  • platform/graphics/gstreamer/GStreamerUtilities.h:
  • platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:

(StreamingClient::createReadBuffer):

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r210583 r210584  
     12017-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GStreamer] Use smart pointers and modernize code in WebKitWebAudioSourceGStreamer
     4        https://bugs.webkit.org/show_bug.cgi?id=166886
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        This patch doesn't change the behavior, so it's covered by existing Web Audio tests. It replaces pointers with
     9        smart pointers, uses WTF::Vector instead of GSList and simplifies the code to map/unmap GstBuffers.
     10
     11        * platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:
     12        (webKitWebAudioSrcConstructed):
     13        (webKitWebAudioSrcFinalize):
     14        (webKitWebAudioSrcLoop):
     15        (webKitWebAudioSrcChangeState):
     16        * platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
     17        (WTF::derefGPtr<GstBufferList>):
     18        (WTF::adoptGRef):
     19        (WTF::refGPtr<GstBufferPool>):
     20        (WTF::derefGPtr<GstBufferPool>):
     21        * platform/graphics/gstreamer/GRefPtrGStreamer.h:
     22        * platform/graphics/gstreamer/GStreamerUtilities.cpp:
     23        (WebCore::mapGstBuffer):
     24        * platform/graphics/gstreamer/GStreamerUtilities.h:
     25        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
     26        (StreamingClient::createReadBuffer):
     27
    1282017-01-11  Commit Queue  <commit-queue@webkit.org>
    229

  • trunk/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp

    r210583 r210584  
    6161    GRecMutex mutex;
    6262
    63     GSList* sources; // List of appsrc. One appsrc for each planar audio channel.
    64     GstPad* sourcePad; // src pad of the element, interleaved wav data is pushed to it.
     63    // List of appsrc. One appsrc for each planar audio channel.
     64    Vector<GRefPtr<GstElement>> sources;
     65
     66    // src pad of the element, interleaved wav data is pushed to it.
     67    GstPad* sourcePad;
    6568
    6669    guint64 numberOfSamples;
    6770
    68     GstBufferPool* pool;
     71    GRefPtr<GstBufferPool> pool;
    6972};
    7073
    … …  
    7679};
    7780
    78 typedef struct {
    79     GstBuffer* buffer;
    80     GstMapInfo info;
    81 } AudioSrcBuffer;
    82 
    8381static GstStaticPadTemplate srcTemplate = GST_STATIC_PAD_TEMPLATE("src",
    8482    GST_PAD_SRC,
    … …  
    221219    for (unsigned channelIndex = 0; channelIndex < priv->bus->numberOfChannels(); channelIndex++) {
    222220        GUniquePtr<gchar> appsrcName(g_strdup_printf("webaudioSrc%u", channelIndex));
    223         GstElement* appsrc = gst_element_factory_make("appsrc", appsrcName.get());
     221        GRefPtr<GstElement> appsrc = gst_element_factory_make("appsrc", appsrcName.get());
    224222        GRefPtr<GstCaps> monoCaps = adoptGRef(getGStreamerMonoAudioCaps(priv->sampleRate));
    225223
    … …  
    230228
    231229        // Configure the appsrc for minimal latency.
    232         g_object_set(appsrc, "max-bytes", static_cast<guint64>(2 * priv->bufferSize), "block", TRUE,
     230        g_object_set(appsrc.get(), "max-bytes", static_cast<guint64>(2 * priv->bufferSize), "block", TRUE,
    233231            "blocksize", priv->bufferSize,
    234232            "format", GST_FORMAT_TIME, "caps", caps.get(), nullptr);
    235233
    236         priv->sources = g_slist_prepend(priv->sources, gst_object_ref(appsrc));
    237 
    238         gst_bin_add(GST_BIN(src), appsrc);
    239         gst_element_link_pads_full(appsrc, "src", priv->interleave.get(), "sink_%u", GST_PAD_LINK_CHECK_NOTHING);
    240     }
    241     priv->sources = g_slist_reverse(priv->sources);
     234        priv->sources.append(appsrc);
     235
     236        gst_bin_add(GST_BIN(src), appsrc.get());
     237        gst_element_link_pads_full(appsrc.get(), "src", priv->interleave.get(), "sink_%u", GST_PAD_LINK_CHECK_NOTHING);
     238    }
    242239
    243240    // interleave's src pad is the only visible pad of our element.
    … …  
    252249
    253250    g_rec_mutex_clear(&priv->mutex);
    254 
    255     g_slist_free_full(priv->sources, reinterpret_cast<GDestroyNotify>(gst_object_unref));
    256251
    257252    priv->~WebKitWebAudioSourcePrivate();
    … …  
    320315    }
    321316
     317    ASSERT(priv->pool);
    322318    GstClockTime timestamp = gst_util_uint64_scale(priv->numberOfSamples, GST_SECOND, priv->sampleRate);
    323319    priv->numberOfSamples += priv->framesToPull;
    324320    GstClockTime duration = gst_util_uint64_scale(priv->numberOfSamples, GST_SECOND, priv->sampleRate) - timestamp;
    325321
    326     GSList* channelBufferList = 0;
    327     for (int i = g_slist_length(priv->sources) - 1; i >= 0; i--) {
    328         AudioSrcBuffer* buffer = g_new(AudioSrcBuffer, 1);
    329         GstBuffer* channelBuffer;
    330 
    331         GstFlowReturn ret = gst_buffer_pool_acquire_buffer(priv->pool, &channelBuffer, nullptr);
    332 
     322    Vector<GRefPtr<GstBuffer>> channelBufferList;
     323    channelBufferList.reserveInitialCapacity(priv->sources.size());
     324    for (unsigned i = 0; i < priv->sources.size(); ++i) {
     325        GRefPtr<GstBuffer> buffer;
     326        GstFlowReturn ret = gst_buffer_pool_acquire_buffer(priv->pool.get(), &buffer.outPtr(), nullptr);
    333327        if (ret != GST_FLOW_OK) {
    334             g_free(buffer);
    335             while (channelBufferList) {
    336                 buffer = static_cast<AudioSrcBuffer*>(channelBufferList->data);
    337                 gst_buffer_unmap(buffer->buffer, &buffer->info);
    338                 gst_buffer_unref(buffer->buffer);
    339                 g_free(buffer);
    340                 channelBufferList = g_slist_delete_link(channelBufferList, channelBufferList);
    341             }
     328            for (auto& buffer : channelBufferList)
     329                unmapGstBuffer(buffer.get());
    342330
    343331            // FLUSHING and EOS are not errors.
    … …  
    348336        }
    349337
    350         ASSERT(channelBuffer);
    351         buffer->buffer = channelBuffer;
    352         GST_BUFFER_TIMESTAMP(channelBuffer) = timestamp;
    353         GST_BUFFER_DURATION(channelBuffer) = duration;
    354         gst_buffer_map(channelBuffer, &buffer->info, (GstMapFlags) GST_MAP_READWRITE);
    355         priv->bus->setChannelMemory(i, reinterpret_cast<float*>(buffer->info.data), priv->framesToPull);
    356         channelBufferList = g_slist_prepend(channelBufferList, buffer);
     338        ASSERT(buffer);
     339        GST_BUFFER_TIMESTAMP(buffer.get()) = timestamp;
     340        GST_BUFFER_DURATION(buffer.get()) = duration;
     341        mapGstBuffer(buffer.get(), GST_MAP_READWRITE);
     342        priv->bus->setChannelMemory(i, reinterpret_cast<float*>(getGstBufferDataPointer(buffer.get())), priv->framesToPull);
     343        channelBufferList.uncheckedAppend(WTFMove(buffer));
    357344    }
    358345
    … …  
    360347    priv->provider->render(0, priv->bus, priv->framesToPull);
    361348
    362     GSList* sourcesIt = priv->sources;
    363     GSList* buffersIt = channelBufferList;
    364 
    365     GstFlowReturn ret = GST_FLOW_OK;
    366     for (int i = 0; sourcesIt && buffersIt; sourcesIt = g_slist_next(sourcesIt), buffersIt = g_slist_next(buffersIt), ++i) {
    367         GstElement* appsrc = static_cast<GstElement*>(sourcesIt->data);
    368         AudioSrcBuffer* buffer = static_cast<AudioSrcBuffer*>(buffersIt->data);
    369         GstBuffer* channelBuffer = buffer->buffer;
    370 
     349    ASSERT(channelBufferList.size() == priv->sources.size());
     350    bool failed = false;
     351    for (unsigned i = 0; i < priv->sources.size(); ++i) {
    371352        // Unmap before passing on the buffer.
    372         gst_buffer_unmap(channelBuffer, &buffer->info);
    373         g_free(buffer);
    374 
    375         if (ret == GST_FLOW_OK) {
    376             ret = gst_app_src_push_buffer(GST_APP_SRC(appsrc), channelBuffer);
    377             if (ret != GST_FLOW_OK) {
    378                 // FLUSHING and EOS are not errors.
    379                 if (ret < GST_FLOW_EOS || ret == GST_FLOW_NOT_LINKED)
    380                     GST_ELEMENT_ERROR(src, CORE, PAD, ("Internal WebAudioSrc error"), ("Failed to push buffer on %s flow: %s", GST_OBJECT_NAME(appsrc), gst_flow_get_name(ret)));
    381                 gst_task_stop(src->priv->task.get());
    382             }
    383         } else
    384             gst_buffer_unref(channelBuffer);
    385     }
    386 
    387     g_slist_free(channelBufferList);
     353        auto& buffer = channelBufferList[i];
     354        unmapGstBuffer(buffer.get());
     355
     356        if (failed)
     357            continue;
     358
     359        auto& appsrc = priv->sources[i];
     360        // Leak the buffer ref, because gst_app_src_push_buffer steals it.
     361        GstFlowReturn ret = gst_app_src_push_buffer(GST_APP_SRC(appsrc.get()), buffer.leakRef());
     362        if (ret != GST_FLOW_OK) {
     363            // FLUSHING and EOS are not errors.
     364            if (ret < GST_FLOW_EOS || ret == GST_FLOW_NOT_LINKED)
     365                GST_ELEMENT_ERROR(src, CORE, PAD, ("Internal WebAudioSrc error"), ("Failed to push buffer on %s flow: %s", GST_OBJECT_NAME(appsrc.get()), gst_flow_get_name(ret)));
     366            gst_task_stop(src->priv->task.get());
     367            failed = true;
     368        }
     369    }
    388370}
    389371
    … …  
    415397    case GST_STATE_CHANGE_READY_TO_PAUSED: {
    416398        GST_DEBUG_OBJECT(src, "READY->PAUSED");
     399
    417400        src->priv->pool = gst_buffer_pool_new();
    418         GstStructure* config = gst_buffer_pool_get_config(src->priv->pool);
     401        GstStructure* config = gst_buffer_pool_get_config(src->priv->pool.get());
    419402        gst_buffer_pool_config_set_params(config, nullptr, src->priv->bufferSize, 0, 0);
    420         gst_buffer_pool_set_config(src->priv->pool, config);
    421         if (!gst_buffer_pool_set_active(src->priv->pool, TRUE))
     403        gst_buffer_pool_set_config(src->priv->pool.get(), config);
     404        if (!gst_buffer_pool_set_active(src->priv->pool.get(), TRUE))
    422405            returnValue = GST_STATE_CHANGE_FAILURE;
    423406        else if (!gst_task_start(src->priv->task.get()))
    … …  
    427410    case GST_STATE_CHANGE_PAUSED_TO_READY:
    428411        GST_DEBUG_OBJECT(src, "PAUSED->READY");
     412
    429413#if GST_CHECK_VERSION(1, 4, 0)
    430         gst_buffer_pool_set_flushing(src->priv->pool, TRUE);
     414        gst_buffer_pool_set_flushing(src->priv->pool.get(), TRUE);
    431415#endif
    432416        if (!gst_task_join(src->priv->task.get()))
    433417            returnValue = GST_STATE_CHANGE_FAILURE;
    434         gst_buffer_pool_set_active(src->priv->pool, FALSE);
    435         gst_object_unref(src->priv->pool);
     418        gst_buffer_pool_set_active(src->priv->pool.get(), FALSE);
    436419        src->priv->pool = nullptr;
    437420        break;

  • trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp

    r202615 r210584  
    222222}
    223223
     224template<> GRefPtr<GstBufferPool> adoptGRef(GstBufferPool* ptr)
     225{
     226    ASSERT(!ptr || !g_object_is_floating(ptr));
     227    return GRefPtr<GstBufferPool>(ptr, GRefPtrAdopt);
     228}
     229
     230template<> GstBufferPool* refGPtr<GstBufferPool>(GstBufferPool* ptr)
     231{
     232    if (ptr)
     233        gst_object_ref_sink(GST_OBJECT(ptr));
     234
     235    return ptr;
     236}
     237
     238template<> void derefGPtr<GstBufferPool>(GstBufferPool* ptr)
     239{
     240    if (ptr)
     241        gst_object_unref(ptr);
     242}
     243
    224244template<> GRefPtr<GstSample> adoptGRef(GstSample* ptr)
    225245{

  • trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h

    r202615 r210584  
    3434typedef struct _GstBuffer GstBuffer;
    3535typedef struct _GstBufferList GstBufferList;
     36typedef struct _GstBufferPool GstBufferPool;
    3637typedef struct _GstSample GstSample;
    3738typedef struct _GstTagList GstTagList;
    … …  
    8485template<> void derefGPtr<GstBufferList>(GstBufferList*);
    8586
     87template<> GRefPtr<GstBufferPool> adoptGRef(GstBufferPool*);
     88template<> GstBufferPool* refGPtr<GstBufferPool>(GstBufferPool*);
     89template<> void derefGPtr<GstBufferPool>(GstBufferPool*);
     90
    8691template<> GRefPtr<GstSample> adoptGRef(GstSample* ptr);
    8792template<> GstSample* refGPtr<GstSample>(GstSample* ptr);

  • trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp

    r210132 r210584  
    121121}
    122122
    123 void mapGstBuffer(GstBuffer* buffer)
     123void mapGstBuffer(GstBuffer* buffer, uint32_t flags)
    124124{
    125125    GstMapInfo* mapInfo = static_cast<GstMapInfo*>(fastMalloc(sizeof(GstMapInfo)));
    126     if (!gst_buffer_map(buffer, mapInfo, GST_MAP_WRITE)) {
     126    if (!gst_buffer_map(buffer, mapInfo, static_cast<GstMapFlags>(flags))) {
    127127        fastFree(mapInfo);
    128128        gst_buffer_unref(buffer);
    … …  
    131131
    132132    GstMiniObject* miniObject = reinterpret_cast<GstMiniObject*>(buffer);
    133     gst_mini_object_set_qdata(miniObject, g_quark_from_static_string(webkitGstMapInfoQuarkString), mapInfo, 0);
     133    gst_mini_object_set_qdata(miniObject, g_quark_from_static_string(webkitGstMapInfoQuarkString), mapInfo, nullptr);
    134134}
    135135

  • trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.h

    r210132 r210584  
    5959GstBuffer* createGstBufferForData(const char* data, int length);
    6060char* getGstBufferDataPointer(GstBuffer*);
    61 void mapGstBuffer(GstBuffer*);
     61void mapGstBuffer(GstBuffer*, uint32_t);
    6262void unmapGstBuffer(GstBuffer*);
    6363bool initializeGStreamer();

  • trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp

    r207190 r210584  
    866866    GstBuffer* buffer = gst_buffer_new_and_alloc(requestedSize);
    867867
    868     mapGstBuffer(buffer);
     868    mapGstBuffer(buffer, GST_MAP_WRITE);
    869869
    870870    WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
Note: See TracChangeset for help on using the changeset viewer.