Summary: | [GStreamer][MSE] Generic main thread notification support | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||
Component: | Platform | Assignee: | Philippe Normand <pnormand> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aboya, calvaris, eocanha, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Philippe Normand
2018-08-16 06:26:18 PDT
Created attachment 347258 [details]
Patch
Comment on attachment 347258 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347258&action=review > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:126 > case MediaSourceSeekToTime: { > - GstStructure* structure = gst_structure_new_empty("seek-needs-data"); > - GstMessage* message = gst_message_new_application(GST_OBJECT(appsrc), structure); > - gst_bus_post(webKitMediaSrc->priv->bus.get(), message); > - GST_TRACE("seek-needs-data message posted to the bus"); > + webKitMediaSrc->priv->notifier->notify(MSEMainThreadNotification::SeekNeedsData, [webKitMediaSrc] { > + seekNeedsDataMainThread(webKitMediaSrc); > + }); > break; > } You don't need to enclose the case in { } anymore since we don't have scoped variables. Please, remove them. > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:143 > if (appsrcStream && appsrcStream->type != WebCore::Invalid) { > - GstStructure* structure = gst_structure_new("ready-for-more-samples", "appsrc-stream", G_TYPE_POINTER, appsrcStream, nullptr); > - GstMessage* message = gst_message_new_application(GST_OBJECT(appsrc), structure); > - gst_bus_post(webKitMediaSrc->priv->bus.get(), message); > - GST_TRACE("ready-for-more-samples message posted to the bus"); > + > + webKitMediaSrc->priv->notifier->notify(MSEMainThreadNotification::ReadyForMoreSamples, [webKitMediaSrc, appsrcStream] { > + notifyReadyForMoreSamplesMainThread(webKitMediaSrc, appsrcStream); > + }); > } You don't need the if { } anymore because there is only one clause in it. Please remove them. > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h:100 > +enum MSEMainThreadNotification { I guess you preprended MSE to this type because it can collide with other types defined inside other other files. In this case I would strongly encourage you to preprend the whole class and name it WebKitMediaSrcMainThreadNotification. > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h:102 > + SeekNeedsData = 1 << 0, > + ReadyForMoreSamples = 1 << 1 Longshot nit: I think it would be nicer to, at least, begin with these alphabetically sorted. It is not a requirement, but nice to have :) LGTM too. Committed r235110: <https://trac.webkit.org/changeset/235110> |