Bug 190683 - [GStreamer][WebRTC] Do not try to handle framerate modulation in the encoder
Summary: [GStreamer][WebRTC] Do not try to handle framerate modulation in the encoder
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: 190682
Blocks: 187064 190684
  Show dependency tree / graph
 
Reported: 2018-10-17 12:57 PDT by Thibault Saunier
Modified: 2018-11-06 06:24 PST (History)
5 users (show)

See Also:

Attachments
Patch (2.94 KB, patch)
2018-10-17 12:58 PDT, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Patch (2.96 KB, patch)
2018-10-17 12:59 PDT, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Patch (2.96 KB, patch)
2018-11-05 11:05 PST, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Patch (2.96 KB, patch)
2018-11-06 04:40 PST, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Thibault Saunier 2018-10-17 12:57:51 PDT 
[GStreamer][WebRTC] Do not try to handle framerate modulation in the encoder
Comment 1 Thibault Saunier 2018-10-17 12:58:07 PDT 
Created attachment 352620 [details]
Patch
Comment 2 Thibault Saunier 2018-10-17 12:59:18 PDT 
Created attachment 352622 [details]
Patch
Comment 3 Thibault Saunier 2018-11-05 11:05:06 PST 
Created attachment 353888 [details]
Patch
Comment 4 Xabier Rodríguez Calvar 2018-11-05 22:56:44 PST 
Comment on attachment 353888 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        This has to already be handled in capturing pipeline or in libwebrtc itself

Period at the end.

> Source/WebCore/ChangeLog:10
> +        No other encoder implementation try to do that, and libwebrtc is not happy with encoder that do not output the exact number of frames that have been passed in

try -> tried, "with encoders that" and period at the end.
Comment 5 Xabier Rodríguez Calvar 2018-11-05 22:58:45 PST 
Comment on attachment 353888 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        No regressions detected and libwebrtc is happier this way, less warning output and no more frame corruption in H264 streams found.

My only concern is we should have a test for this, I'll let Phil decide.
Comment 6 Thibault Saunier 2018-11-06 04:15:43 PST 
(In reply to Xabier Rodríguez Calvar from comment #5)
> Comment on attachment 353888 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=353888&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        No regressions detected and libwebrtc is happier this way, less warning output and no more frame corruption in H264 streams found.
> 
> My only concern is we should have a test for this, I'll let Phil decide.

Well, we basically have "test", libwebrtc is not throwing WARNINGS anymore, I do not see how we could have a dedicated test at our level.
Comment 7 Thibault Saunier 2018-11-06 04:40:20 PST 
Created attachment 353962 [details]
Patch
Comment 8 WebKit Commit Bot 2018-11-06 06:23:32 PST 
Comment on attachment 353962 [details]
Patch

Clearing flags on attachment: 353962

Committed r237860: <https://trac.webkit.org/changeset/237860>
Comment 9 WebKit Commit Bot 2018-11-06 06:23:33 PST 
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-11-06 06:24:22 PST 
<rdar://problem/45841276>