Bug 218748 - [GStreamer] refactor video encoder with WebKit style
Summary: [GStreamer] refactor video encoder with WebKit style
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Víctor M. Jáquez L.
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-10 06:02 PST by Víctor M. Jáquez L.
Modified: 2020-11-27 02:23 PST (History)
14 users (show)

See Also:


Attachments
Patch (25.89 KB, patch)
2020-11-10 07:27 PST, Víctor M. Jáquez L.
no flags Details | Formatted Diff | Diff
Patch (25.95 KB, patch)
2020-11-23 03:58 PST, Víctor M. Jáquez L.
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Víctor M. Jáquez L. 2020-11-10 06:02:07 PST
libwebrtc/GStreamerVideoEncoder is written in a GStreamer code-style. It would be better to refactor it using C++/WebKit style, as the other GStreamer elements inside WebKit.
Comment 1 Víctor M. Jáquez L. 2020-11-10 07:27:06 PST
Created attachment 413694 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2020-11-11 03:44:56 PST
Let's wait for Phil and Thibault's comments, since IIRC, it was their idea to keep it this way to ease a possible upstreaming process to GStreamer eventually.
Comment 3 Thibault Saunier 2020-11-11 04:33:09 PST
(In reply to Xabier Rodríguez Calvar from comment #2)
> Let's wait for Phil and Thibault's comments, since IIRC, it was their idea
> to keep it this way to ease a possible upstreaming process to GStreamer
> eventually.

It indeed was the plan and the reason why we used the GStreamer coding style. I never took the time to make it happen and am not 100% sure what other GStreamer maintainer will think of it so I have no hard opinion at that point.
Comment 4 Philippe Normand 2020-11-23 01:32:44 PST
Comment on attachment 413694 [details]
Patch

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

> Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoder.cpp:39
> +    GST_STATIC_PAD_TEMPLATE ("src", GST_PAD_SRC, GST_PAD_ALWAYS, GST_STATIC_CAPS ("video/x-h264;video/x-vp8;video/x-vp9"));

vp9 is added here, but I see no mention of a vp9enc below. Maybe add vp9 in a follow-up patch, if this patch is meant to be a refactoring only.
Comment 5 Víctor M. Jáquez L. 2020-11-23 03:58:03 PST
Created attachment 414799 [details]
Patch
Comment 6 EWS 2020-11-27 02:22:08 PST
Committed r270187: <https://trac.webkit.org/changeset/270187>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414799 [details].
Comment 7 Radar WebKit Bug Importer 2020-11-27 02:23:16 PST
<rdar://problem/71761576>