Bug 230881 - Have SourceBufferPrivateAVJObjC use WorkQueue instead of dispatch_async
Summary: Have SourceBufferPrivateAVJObjC use WorkQueue instead of dispatch_async
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar
: 230663 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-09-28 00:50 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-10-06 17:43 PDT (History)
6 users (show)

See Also:


Attachments
Patch (11.86 KB, patch)
2021-09-28 01:13 PDT, Jean-Yves Avenard [:jya]
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (11.88 KB, patch)
2021-09-28 01:23 PDT, Jean-Yves Avenard [:jya]
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (12.13 KB, patch)
2021-09-28 04:12 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (12.43 KB, patch)
2021-10-05 00:10 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (5.91 KB, patch)
2021-10-05 21:41 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].