Merge master into staging. by charlie-ht · Pull Request #369 · WebPlatformForEmbedded/WPEWebKit

charlie-ht · 2017-09-11T18:05:02Z

Please let me know what tests I should run, I tried running some tests with a debug build but that seems hopeless on both master and staging.

Commits merged

[MSE] Fix BufUnbuf test from YouTube's MSE conformance test suite
06b35a8
[MSE] Improve SourceBuffer::appendBuffer() time
e5b8d88
[MSE] Improve perfomance of SourceBuffer::evictCodedFrames
ce724e5
[MSE] fix calculation of MediaSource::currentTimeFudgeFactor
26a5288
Cap the reported progress of implicit animations
7162d05
[MSE] Fix unsigned comparison warning.
aec09ae
[GStreamer][MSE] Make setAppendState thread safe
4508db9
[WPE] Enable MEMORY_SAMPLER
26614e8
Revert "[GStreamer][MSE] Add missing lock around getStreamByTrackId"
aaab302
Use the return value of gst_mini_object_make_writable...
b4df1c5
webbrige sets WPE_RAM_SIZE to 128M, which tickles a logic error in
the cache model.
066c52b
Fixed UI issues in webinspector
b0e7fd8
Fix for issue #326 WebInspector-Popover
9cbbdd7
[WPEWebkit] Added support for ARMV8 processor.
d3a3365
[GStreamer] Fix video resize problems after quality change
b2ce7dc
Fix for PageUp/PageDown keys
29c10d9
[Gstreamer] Make sure live stream is detected
ef9a398
Fix for WebInspector-NetworkTab
71d0d30
[GStreamer] Fix for invalid video size information sometimes returned by getVideoSizeAndFormatFromCaps (gst_video_info_from_caps)
59fbb57
[GStreamer] Make volume control consistent
939aa76
[MSE][GStreamer] Fix premature video clip change on youtube
8dfa02e
[GStreamer][MSE] Don't flush audio streams, only video streams
148ffdd
Fixed bug with unneeded rangeRemoval
25c3dd3
[MSE][GStreamer] don't pause playback when there is enough data
9842bc6
[MSE][GStreamer] increase the playback buffer size
72a5ffc
Fixed UI issues in webinspector
3f777e3
Fix for wpe PageClient overrides
5000203
Fixed MediaElementsEvents test
97c32be
[MSE][GStreamer] Fix infinite loop in evictCodedFrames() when duration is infinite
c11d0d4
Fixed typo indefiniteTime -> isIndefinite (#306)
a4cc739
[MSE] [GStreamer] Fixing QUOTA_EXCEEDED_ERR fired when there was evictable data in Source Buffer (#296)
eec35dd
minor compile errors [sic]
d392c15
volume method wrapped with WPE, usage is not
4e9b1cf
Add a METROLOGICAL define to be able to ifdef pieces of code specific to downstream WPE
fdac418
Implemented custom headers per url
21f3896
Made fudge factor for audio a fraction
c527870
Fix MSE video size test failures (#276) (#277)
277e5de
[GStreamer][MSE] Reject too big channels/framerate/width/height mime type values

Commits we might want but that conflict heavily or seem N/A.

254e68d
Make scroll-to-focused-element behaviour configurable.
d608ad3
Add header for std::functional
758f885
Fixed bug with wrong expires date in cookies
11727e2
[MSE][GStreamer] keep the ahead buffer when evicting coded frames
7fb21a9
[GStreamer][MSE] Fix stall in seek to end of buffered range
3cd4d31
[GStreamer][MSE] Fix stall in seek due to incomplete reenqueue
b407c23 WKView: add a wpe_view_backend getter.
c99d5ee
[GStreamer] Notify player private (PlatformLayer) destruction to the
d9f7dc6
Fix-for-delay-in-web-audio
06ff542
Generalise DB410c switch
427b052
[MSE] Don't try to adopt a floater.
259e898
Fix for 302 Redirect support (#356)
4e778b5
[WebInspector] Fix for elements page, use getPropertyValue instead of discontinued getPropertyCSSValue
313ff46
Add platform variable for DB410c
f017f70
Using optimised vsink for platform DB410C
9a02007
[GTK][WPE] border-radius with non visible border doesn't work on images that have their own RenderLayer
5674f3b
[GStreamer][WebAudio] Avoid sending null data/silence
83a08d0
Add a page property for enabling console log with private browsing
e2c897e
Use -ffunction-sections/-fdata-sections/--gc-sections with clang too [where has this stuff gone?]
cad2f3c
Make Web Inspector Remote to listen on both IPv4 and IPv6 addresses
85504c3
Make the code inclusive of musl when OS is linux
ae1b1e5
Use a setting to disable scrolling to the keyboard focused element instead of an env variable.

calvaris

We need to fix these things. staging is close to upstream meaning we want to respect style even more. I don't think you need to to fix these things in every commit where they happened, just a review commit at the end.

Also, if the changes I am talking about are needed upstream we should file bugs upstream.

Besides, I guess @eocanha, @zdobersek and @carlosgcampos should have a quick look at this too.

calvaris · 2017-09-12T06:52:53Z

+ trackBuffer.lastEnqueuedDecodeEndTime.isValid() &&
+ trackBuffer.lastDecodeTimestamp.isValid() &&
+ abs(trackBuffer.lastEnqueuedDecodeEndTime - trackBuffer.lastDecodeTimestamp) > MediaTime::createWithDouble(0.350) )
+ {


This pair if {} should not be added.

calvaris · 2017-09-12T06:54:19Z

 if (!getVideoSizeAndFormatFromCaps(caps.get(), originalSize, format, pixelAspectRatioNumerator, pixelAspectRatioDenominator, stride))
 return FloatSize();

+ // Sanity check for the unlikely, but reproducible case when getVideoSizeAndFormatFromCaps returns incorrect values


Period at the end of the comment.

calvaris · 2017-09-12T07:10:45Z

+ if (info)
+ infoCopy = gst_structure_copy(info);
+ m_sample = adoptGRef(gst_sample_new(nullptr, gst_sample_get_caps(m_sample.get()),
+ gst_sample_get_segment(m_sample.get()), infoCopy));


info ? gst_structure_copy(info) : nullptr and we can avoid the infoCopy variable hassle

calvaris · 2017-09-12T07:12:04Z

+ "transition",
+ G_TYPE_INT,
+ AppendState::KeyNegotiation,
+ nullptr);


This can be done in one line

calvaris · 2017-09-12T07:15:19Z

+ if (!isTimeBuffered(seekTime) || m_readyState < MediaPlayer::HaveCurrentData) {
+ // Media source may trigger seek completion even when the target time is not yet buffered,
+ // in this case it is better continue the seek and wait for the app to provide media data.
+ m_mseSeekCompleted=true;


Need spaces around =

calvaris · 2017-09-12T07:46:34Z


+ // Sanity check for the unlikely, but reproducible case when getVideoSizeAndFormatFromCaps returns incorrect values
+ if ((originalSize.width() == 0) || (originalSize.height() == 0)
+ || (pixelAspectRatioNumerator == 0) || (pixelAspectRatioNumerator == 0)) {


Numeric comparisons == 0 should be done as ! .

calvaris · 2017-09-12T07:47:43Z

+ if (!g_strcmp0(contextType, "drm-preferred-decryption-system-id")) {
+ if (WTF::isMainThread()) {
+ transitionTo(AppendState::KeyNegotiation);
+ } else {


{ } not needed here.

calvaris · 2017-09-12T07:49:09Z

 #include "MediaSample.h"
 #include <gst/gst.h>
 #include <wtf/text/AtomicString.h>
+#include <wtf/PrintStream.h>


You need to sort these two includes.

Didn't the style checker complain about this?

calvaris · 2017-09-12T07:54:16Z


 GstContext* context = gst_context_new("http-headers", FALSE);
- gst_context_make_writable(context);
+context = gst_context_make_writable(context);


calvaris · 2017-09-12T07:58:02Z

+
 MediaTime MediaPlayerPrivateGStreamerMSE::currentMediaTime() const
 {
+ MediaTime cachedPosition = MediaTime::createWithFloat(m_cachedPosition);


no float here!

That would require refactoring the m_cachedPosition usage (my media time patch never made it into these branches, or upstream). I'm not going to refactor this while merging.

Ok, but please, do not lose track of that cause I think it is an important change.

calvaris

Very nice style catches. We are much closer!

calvaris · 2017-09-13T06:01:09Z

 trackBuffer.lastDecodeTimestamp.isValid() &&
- abs(trackBuffer.lastEnqueuedDecodeEndTime - trackBuffer.lastDecodeTimestamp) > MediaTime::createWithDouble(0.350) )
- {
+ abs(trackBuffer.lastEnqueuedDecodeEndTime - trackBuffer.lastDecodeTimestamp) > MediaTime::createWithDouble(0.350)) {


It seems that for this, { } are not needed.

calvaris · 2017-09-13T06:02:26Z

 #include "MediaSample.h"
 #include <gst/gst.h>
 #include <wtf/text/AtomicString.h>
+#include <wtf/PrintStream.h>


Didn't the style checker complain about this?

calvaris · 2017-09-13T06:03:01Z

 GST_DEBUG_OBJECT(m_webKitMediaSrc.get(), "State %d", int(GST_STATE(m_webKitMediaSrc.get())));

- Stream* stream = new Stream{ };
+ Stream *stream = new Stream{ };


calvaris · 2017-09-13T06:08:03Z

+ return;
+ }
+
+ auto buffered = m_buffered->ranges();


Better auto&

calvaris · 2017-09-13T06:19:22Z

+ if (!originalSize.width() || !originalSize.height()
+ || !pixelAspectRatioNumerator || !pixelAspectRatioNumerator) {
+ GST_DEBUG("getVideoSizeAndFormatFromCaps returned an invalid info, returning an empty size");
+ return FloatSize();


We can return { }.

calvaris · 2017-09-13T06:21:06Z

- for (auto& sourceBuffer : *m_sourceBuffers)
- sourceBuffer->rangeRemoval(newDuration, m_duration);
+ for (auto& sourceBuffer : *m_sourceBuffers) {
+ auto length = sourceBuffer->bufferedInternal().length();


I think the auto here is extreme lazyness. We can go for unsigned.

calvaris · 2017-09-13T06:23:18Z

 MediaEngineSupportParameters parameters;
 parameters.type = contentType;
+ parameters.channels = channels;
+ parameters.dimension = FloatSize(width, height);


You can probably use the { } constructor here.

Commits merged, [MSE] Fix BufUnbuf test from YouTube's MSE conformance test suite 06b35a8 [MSE] Improve SourceBuffer::appendBuffer() time e5b8d88 [MSE] Improve perfomance of SourceBuffer::evictCodedFrames ce724e5 [MSE] fix calculation of MediaSource::currentTimeFudgeFactor 26a5288 Cap the reported progress of implicit animations 7162d05 [MSE] Fix unsigned comparison warning. aec09ae [GStreamer][MSE] Make setAppendState thread safe 4508db9 [WPE] Enable MEMORY_SAMPLER 26614e8 Revert "[GStreamer][MSE] Add missing lock around getStreamByTrackId" aaab302 Use the return value of gst_mini_object_make_writable... b4df1c5 webbrige sets WPE_RAM_SIZE to 128M, which tickles a logic error in the cache model. 066c52b Fixed UI issues in webinspector b0e7fd8 Fix for issue #326 WebInspector-Popover 9cbbdd7 [WPEWebkit] Added support for ARMV8 processor. d3a3365 [GStreamer] Fix video resize problems after quality change b2ce7dc Fix for PageUp/PageDown keys 29c10d9 [Gstreamer] Make sure live stream is detected ef9a398 Fix for WebInspector-NetworkTab 71d0d30 [GStreamer] Fix for invalid video size information sometimes returned by getVideoSizeAndFormatFromCaps (gst_video_info_from_caps) 59fbb57 [GStreamer] Make volume control consistent 939aa76 [MSE][GStreamer] Fix premature video clip change on youtube 8dfa02e [GStreamer][MSE] Don't flush audio streams, only video streams 148ffdd Fixed bug with unneeded rangeRemoval 25c3dd3 [MSE][GStreamer] don't pause playback when there is enough data 9842bc6 [MSE][GStreamer] increase the playback buffer size 72a5ffc Fixed UI issues in webinspector 3f777e3 Fix for wpe PageClient overrides 5000203 Fixed MediaElementsEvents test 97c32be [MSE][GStreamer] Fix infinite loop in evictCodedFrames() when duration is infinite c11d0d4 Fixed typo indefiniteTime -> isIndefinite (#306) a4cc739 [MSE] [GStreamer] Fixing QUOTA_EXCEEDED_ERR fired when there was evictable data in Source Buffer (#296) eec35dd minor compile errors [sic] d392c15 volume method wrapped with WPE, usage is not 4e9b1cf Add a METROLOGICAL define to be able to ifdef pieces of code specific to downstream WPE fdac418 Implemented custom headers per url 21f3896 Made fudge factor for audio a fraction c527870 Fix MSE video size test failures (#276) (#277) 277e5de [GStreamer][MSE] Reject too big channels/framerate/width/height mime type values Commits we might want but that conflict heavily or seem N/A. 254e68d Make scroll-to-focused-element behaviour configurable. d608ad3 Add <functional> header for std::functional 758f885 Fixed bug with wrong expires date in cookies 11727e2 [MSE][GStreamer] keep the ahead buffer when evicting coded frames 7fb21a9 [GStreamer][MSE] Fix stall in seek to end of buffered range 3cd4d31 [GStreamer][MSE] Fix stall in seek due to incomplete reenqueue b407c23 WKView: add a wpe_view_backend getter. c99d5ee [GStreamer] Notify player private (PlatformLayer) destruction to the d9f7dc6 Fix-for-delay-in-web-audio 06ff542 Generalise DB410c switch 427b052 [MSE] Don't try to adopt a floater. 259e898 Fix for 302 Redirect support (#356) 4e778b5 [WebInspector] Fix for elements page, use getPropertyValue instead of discontinued getPropertyCSSValue 313ff46 Add platform variable for DB410c f017f70 Using optimised vsink for platform DB410C 9a02007 [GTK][WPE] border-radius with non visible border doesn't work on images that have their own RenderLayer 5674f3b [GStreamer][WebAudio] Avoid sending null data/silence 83a08d0 Add a page property for enabling console log with private browsing e2c897e Use -ffunction-sections/-fdata-sections/--gc-sections with clang too [where has this stuff gone?] cad2f3c Make Web Inspector Remote to listen on both IPv4 and IPv6 addresses 85504c3 Make the code inclusive of musl when OS is linux ae1b1e5 Use a setting to disable scrolling to the keyboard focused element instead of an env variable.

These are from calvaris and the check-webkit-style, ignoring all warnings less than category level 5, because there's too many to fix (and calvaris didn't notice most of them :))

charlie-ht · 2017-09-13T18:27:38Z

I rebased over @zdobersek merge today and fixed the conflicts, also hope I've addressed the style comments now.

@calvaris , eme-v3.html test is unaffected by this merge.

The following is now silent, ./Tools/Scripts/check-webkit-style -g 90bc784..

calvaris

If this does not break anything, we're good to go.

charlie-ht · 2017-09-14T08:35:31Z

Thanks for the reviews calvaris.

charlie-ht requested review from calvaris and zdobersek September 11, 2017 18:05

calvaris requested changes Sep 12, 2017
View reviewed changes

calvaris requested changes Sep 13, 2017
View reviewed changes

charlie-ht added 2 commits September 13, 2017 16:20

Fix style comments.
4cab5bc
These are from calvaris and the check-webkit-style, ignoring all warnings less than category level 5, because there's too many to fix (and calvaris didn't notice most of them :))

charlie-ht force-pushed the cturner/master-staging-merge branch from 10a7570 to 793fffbCompare September 13, 2017 18:26

Fix more style issues.
65d4dad
The following is now silent, ./Tools/Scripts/check-webkit-style -g 90bc784..

charlie-ht force-pushed the cturner/master-staging-merge branch from 793fffb to 65d4dadCompare September 13, 2017 18:33

calvaris approved these changes Sep 14, 2017
View reviewed changes

charlie-ht merged commit 448d11f into staging-emev3Sep 14, 2017

charlie-ht deleted the cturner/master-staging-merge branch March 6, 2019 10:35

Uh oh!

Conversation

charlie-ht commented Sep 11, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commits merged

Commits we might want but that conflict heavily or seem N/A.

Uh oh!

calvaris left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

calvarisSep 12, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

calvaris left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlie-ht commented Sep 13, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calvaris left a comment

Choose a reason for hiding this comment

Uh oh!

charlie-ht commented Sep 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

charlie-ht commented Sep 11, 2017•
edited
Loading

calvaris left a comment •
edited
Loading

calvarisSep 12, 2017•
edited
Loading

charlie-ht commented Sep 13, 2017•
edited
Loading