Bug 230881

Summary: Have SourceBufferPrivateAVJObjC use WorkQueue instead of dispatch_async
Product: WebKit Reporter: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Component: MediaAssignee: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, eric.carlson, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=230884
https://bugs.webkit.org/show_bug.cgi?id=230662
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch none

Description Jean-Yves Avenard [:jya] 2021-09-28 00:50:27 PDT
Have SourceBufferPrivateAVJObjC use WorkQueue instead of dispatch_async
Comment 1 Jean-Yves Avenard [:jya] 2021-09-28 01:13:24 PDT
Created attachment 439440 [details]
Patch

Hopefully no error in Windows / Linux, can't compile locally
Comment 2 Jean-Yves Avenard [:jya] 2021-09-28 01:23:40 PDT
Created attachment 439442 [details]
Patch
Comment 3 Jean-Yves Avenard [:jya] 2021-09-28 04:12:47 PDT
Created attachment 439453 [details]
Patch
Comment 4 Jean-Yves Avenard [:jya] 2021-09-28 05:27:20 PDT
rdar://problem/83429276
Comment 5 Jean-Yves Avenard [:jya] 2021-09-28 05:28:14 PDT
*** Bug 230663 has been marked as a duplicate of this bug. ***
Comment 6 Jean-Yves Avenard [:jya] 2021-10-05 00:10:19 PDT
Created attachment 440171 [details]
Patch

Add additional explanation in Changelog
Comment 7 Chris Dumez 2021-10-05 07:38:07 PDT
Comment on attachment 440171 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440171&action=review

> Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:59
> +    dispatch_group_enter(m_dispatchGroup.get());

I don't think this is the right level of abstraction and I don't think we want to pay the cost of dispatch_group_enter / dispatch_group_leave for *every* dispatch call, simply to support awaitIdle() at one call site.

If we really wanted to support this, I think we would introduce the concept of group, like in the dispatch API. That said, for this patch, I don't think we need the concept of group or awaitIdle() at all.

> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:700
> +    m_appendQueue->awaitIdle();

Since you have a dedicated queue now, why can't you simply m_appendQueue->dispatchSync() to make sure all previous tasks have completed and hang while waiting?

The previous code was using the concept of group because we were using a shared global queue.
Comment 8 Jean-Yves Avenard [:jya] 2021-10-05 21:41:43 PDT
Created attachment 440328 [details]
Patch
Comment 9 EWS 2021-10-06 17:43:17 PDT
Committed r283685 (242621@main): <https://commits.webkit.org/242621@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440328 [details].