Bug 191861 - [GStreamer][WebRTC] Use LibWebRTC provided vp8 decoders and encoders
Summary: [GStreamer][WebRTC] Use LibWebRTC provided vp8 decoders and encoders
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thibault Saunier
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-20 06:31 PST by Thibault Saunier
Modified: 2018-11-27 09:35 PST (History)
4 users (show)

See Also:


Attachments
Patch (19.59 KB, patch)
2018-11-20 06:32 PST, Thibault Saunier
no flags Details | Formatted Diff | Diff
Patch (19.64 KB, patch)
2018-11-20 06:39 PST, Thibault Saunier
no flags Details | Formatted Diff | Diff
Minor ChangeLog enhancement (19.73 KB, patch)
2018-11-22 04:41 PST, Thibault Saunier
no flags Details | Formatted Diff | Diff
Patch for landing (19.84 KB, patch)
2018-11-27 08:53 PST, Thibault Saunier
no flags Details | Formatted Diff | Diff
Patch for landing (19.85 KB, patch)
2018-11-27 08:57 PST, Thibault Saunier
no flags Details | Formatted Diff | Diff

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