Bug 191861

Summary: [GStreamer][WebRTC] Use LibWebRTC provided vp8 decoders and encoders
Product: WebKit Reporter: Thibault Saunier <tsaunier>
Component: New BugsAssignee: Thibault Saunier <tsaunier>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, pnormand, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Minor ChangeLog enhancement
none
Patch for landing
none
Patch for landing none

Description Thibault Saunier 2018-11-20 06:31:50 PST
[GStreamer][WebRTC] Use libvpx provided decoders and encoders
Comment 1 Thibault Saunier 2018-11-20 06:32:08 PST
Created attachment 355338 [details]
Patch
Comment 2 EWS Watchlist 2018-11-20 06:33:50 PST
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.
Comment 3 Thibault Saunier 2018-11-20 06:39:09 PST
Created attachment 355339 [details]
Patch
Comment 4 Philippe Normand 2018-11-22 01:19:35 PST
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?
Comment 5 Thibault Saunier 2018-11-22 02:45:32 PST
(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.
Comment 6 Thibault Saunier 2018-11-22 04:41:19 PST
Created attachment 355467 [details]
Minor ChangeLog enhancement
Comment 7 Philippe Normand 2018-11-22 06:09:07 PST
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 :)
Comment 8 Thibault Saunier 2018-11-27 08:53:02 PST
Created attachment 355737 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2018-11-27 08:55:26 PST
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
Comment 10 Thibault Saunier 2018-11-27 08:57:14 PST
Created attachment 355738 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2018-11-27 09:34:35 PST
Comment on attachment 355738 [details]
Patch for landing

Clearing flags on attachment: 355738

Committed r238557: <https://trac.webkit.org/changeset/238557>
Comment 12 WebKit Commit Bot 2018-11-27 09:34:37 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2018-11-27 09:35:46 PST
<rdar://problem/46278365>