Bug 191861 – [GStreamer][WebRTC] Use LibWebRTC provided vp8 decoders and encoders

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Thibault Saunier]

[GStreamer][WebRTC] Use libvpx provided decoders and encoders

[Thibault Saunier]

Created attachment 355338 [details]
Patch

[EWS Watchlist]

Attachment 355338 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thibault Saunier]

Created attachment 355339 [details]
Patch

[Philippe Normand]

Comment on attachment 355339 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        The GStreamer implementations are less feature full and less tested, now that Apple
> +        also use the LibWebRTC provided implementations it makes a lot of sense for us to
> +        do the same.

What's missing in the GStreamer implementations? How much work would it be to fix them?

[Thibault Saunier]

(In reply to Philippe Normand from comment #4)
> Comment on attachment 355339 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355339&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        The GStreamer implementations are less feature full and less tested, now that Apple
> > +        also use the LibWebRTC provided implementations it makes a lot of sense for us to
> > +        do the same.
> 
> What's missing in the GStreamer implementations? How much work would it be
> to fix them?

Basically everything related to temporal scalability which is not implemented in GStreamer, pexpi have an implementation here: https://github.com/pexip/gst-plugins-good/commits/master/ext/vpx that we will to upstream at some point :-)

It is quite some work, and tbh I think we should use GStreamer based hardware elements only, there is no good for using them in case LibWebRTC already provides wrapper around the same API as what the GSTreamer element does.

[Thibault Saunier]

Created attachment 355467 [details]
Minor ChangeLog enhancement

[Philippe Normand]

Comment on attachment 355467 [details]
Minor ChangeLog enhancement

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:142
> +    GstMappedFrame(GstBuffer *buffer, GstVideoInfo info, GstMapFlags flags)

Nit: misplaced *s in all this class

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:24
> +#include<thread>

I didn't know the space is optional in #includes :)

[Thibault Saunier]

Created attachment 355737 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 355737 [details]
Patch for landing

Rejecting attachment 355737 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 355737, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/ThirdParty/libwebrtc/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/10167810

[Thibault Saunier]

Created attachment 355738 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 355738 [details]
Patch for landing

Clearing flags on attachment: 355738

Committed r238557: <https://trac.webkit.org/changeset/238557>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46278365>