202204 2019-09-25 07:44:54 -0700 REGRESSION(r246399): [GStreamer] Problems playing AAC streams 2019-11-20 07:33:14 -0800 1 1 1 Unclassified WebKit WebKitGTK Other Unspecified Unspecified RESOLVED FIXED P2 Normal --- 198377 1 berto pnormand bugs-noreply calvaris cgarcia clopez cturner ews-watchlist gustavo menard pnormand vjaquez oldest_to_newest 1574053 0 berto 2019-09-25 07:44:54 -0700 This stream fails to play, or plays only for a few seconds: http://tmp.xaq.nl/test-aac.html These two work fine, but will occasionally fail if played *after* the AAC stream: http://tmp.xaq.nl/test-mp3.html http://tmp.xaq.nl/test-hls.html Reportedly WebKitGTK 2.24.0 was the last one that didn't have problems with this. I can reproduce the problem with 2.24.4, and it seems to fail with 2.26.1 as well. Originally reported here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=941137 1575300 1 clopez 2019-09-30 13:08:16 -0700 I can't reproduce with 2.24.2, but I can with 2.24.3. I can also reproduce with trunk. So I guess the problem is caused by one of the patches back-ported to 2.24.3. Looking at the list https://trac.webkit.org/wiki/WebKitGTK/2.24.x#Proposedmergesfor2.24.3 I see several GStreamer related patches there. 1575302 2 clopez 2019-09-30 13:15:21 -0700 It seems it is caused by r246399 If I revert it on my local checkout (I'm on trunk@250519) I can't longer reproduce the issue. 1575459 3 pnormand 2019-10-01 00:53:50 -0700 I'll try to check it this week, thanks for the bisect Carlos! 1575499 4 pnormand 2019-10-01 05:06:43 -0700 Strange that commit introduced the regression for sure? Here what I observe is that the internal queue gets drained just before the next data chunk arrives and that triggers EOS. However it shouldn't be triggered because this is a "live" non-seekable stream. I have a simple fix but I'll try to write a layout test as well, which will delay the upstreaming of the fix. So here's the fix, in case downstream users are in urgent need of it: diff --git a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp index 9810c257b37..965b6e276d0 100644 --- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp @@ -451,11 +451,11 @@ static GstFlowReturn webKitWebSrcCreate(GstPushSrc* pushSrc, GstBuffer** buffer) GST_BUFFER_OFFSET(*buffer) = baseSrc->segment.position; GST_BUFFER_OFFSET_END(*buffer) = GST_BUFFER_OFFSET(*buffer) + size; GST_TRACE_OBJECT(src, "Buffer bounds set to %" G_GUINT64_FORMAT "-%" G_GUINT64_FORMAT, GST_BUFFER_OFFSET(*buffer), GST_BUFFER_OFFSET_END(*buffer)); - GST_TRACE_OBJECT(src, "doesHaveEOS: %s, wasSeeking: %s, seeking: %s, size: %u", boolForPrinting(priv->doesHaveEOS), boolForPrinting(priv->wasSeeking), boolForPrinting(priv->isSeeking), size); + GST_TRACE_OBJECT(src, "doesHaveEOS: %s, wasSeeking: %s, seeking: %s, buffer size: %u, size: %" G_GUINT64_FORMAT, boolForPrinting(priv->doesHaveEOS), boolForPrinting(priv->wasSeeking), boolForPrinting(priv->isSeeking), size, priv->size); if (priv->haveSize && GST_BUFFER_OFFSET_END(*buffer) >= priv->size) { if (priv->wasSeeking) priv->wasSeeking = false; - else + else if (priv->isSeekable) priv->doesHaveEOS = true; } else if (priv->wasSeeking) priv->wasSeeking = false; 1575506 5 clopez 2019-10-01 05:30:33 -0700 (In reply to Philippe Normand from comment #4) > Strange that commit introduced the regression for sure? > I'm not sure if it was the one that introduced the regression, or the one that seems to work-around the issue if reverted. I didn't a full bisect. I just tested to revert it on my local branch, and observed that after reverting it the issue seems fixed. > Here what I observe is that the internal queue gets drained just before the > next data chunk arrives and that triggers EOS. However it shouldn't be > triggered because this is a "live" non-seekable stream. I have a simple fix > but I'll try to write a layout test as well, which will delay the > upstreaming of the fix. > > So here's the fix, in case downstream users are in urgent need of it: > > diff --git > a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp > b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp > index 9810c257b37..965b6e276d0 100644 > --- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp > +++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp > @@ -451,11 +451,11 @@ static GstFlowReturn webKitWebSrcCreate(GstPushSrc* > pushSrc, GstBuffer** buffer) > GST_BUFFER_OFFSET(*buffer) = baseSrc->segment.position; > GST_BUFFER_OFFSET_END(*buffer) = GST_BUFFER_OFFSET(*buffer) + > size; > GST_TRACE_OBJECT(src, "Buffer bounds set to %" G_GUINT64_FORMAT > "-%" G_GUINT64_FORMAT, GST_BUFFER_OFFSET(*buffer), > GST_BUFFER_OFFSET_END(*buffer)); > - GST_TRACE_OBJECT(src, "doesHaveEOS: %s, wasSeeking: %s, > seeking: %s, size: %u", boolForPrinting(priv->doesHaveEOS), > boolForPrinting(priv->wasSeeking), boolForPrinting(priv->isSeeking), size); > + GST_TRACE_OBJECT(src, "doesHaveEOS: %s, wasSeeking: %s, > seeking: %s, buffer size: %u, size: %" G_GUINT64_FORMAT, > boolForPrinting(priv->doesHaveEOS), boolForPrinting(priv->wasSeeking), > boolForPrinting(priv->isSeeking), size, priv->size); > if (priv->haveSize && GST_BUFFER_OFFSET_END(*buffer) >= > priv->size) { > if (priv->wasSeeking) > priv->wasSeeking = false; > - else > + else if (priv->isSeekable) > priv->doesHaveEOS = true; > } else if (priv->wasSeeking) > priv->wasSeeking = false; I tested this patch and it also fixes the issue :) 1577368 6 380332 pnormand 2019-10-07 09:11:03 -0700 Created attachment 380332 Patch 1577369 7 pnormand 2019-10-07 09:12:33 -0700 I haven't managed to write a reliable test unfortunately. 1578122 8 pnormand 2019-10-09 01:18:00 -0700 Committed r250901: <https://trac.webkit.org/changeset/250901> 1591978 9 berto 2019-11-20 05:57:30 -0800 This may not have been fixed after all (or we have a different bug). Here's a new test case: http://tmp.xaq.nl/test-aac.html Leave it playing normally, and after around 2 minutes of playback you'll notice some hiccups and it will eventually stop completely. Reproduced with the MiniBrowser and WebKitGTK 2.26.2 --- In case the URL I pasted earlier changes, the test case contains this single <audio> element: <audio id="player" controls> <source src="http://icecast.omroep.nl/radio4-eigentijds-aac" type="audio/aac" /> <p>Your browser does not support the audio element.</p> </audio> 1591981 10 pnormand 2019-11-20 06:26:10 -0800 I can't reproduce this issue in trunk. It happens in Ephy TP though, it's not related with EOS, so it's a new bug. Please file it separately and attach logs, as usual with media bugs: GST_DEBUG_FILE=gst.log GST_DEBUG="3,webkit*:8" 1591994 11 berto 2019-11-20 07:33:14 -0800 New bug filed as https://bugs.webkit.org/show_bug.cgi?id=204410 380332 2019-10-07 09:11:03 -0700 2019-10-08 03:21:56 -0700 Patch bug-202204-20191007171102.patch text/plain 2823 pnormand U3VidmVyc2lvbiBSZXZpc2lvbjogMjUwNzcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTg2MmFjZGIyODdlYTlj YzU2MjUyMjBmMTgwYzczZDgzMTIyMjFhMi4uYzMxYmIxZTY1M2I2Y2JmN2Q2ZDRjNzM5NjIzMDVi ZGViYmE0MGRhMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDE5LTEwLTA3ICBQaGls aXBwZSBOb3JtYW5kICA8cG5vcm1hbmRAaWdhbGlhLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9O KHIyNDYzOTkpOiBbR1N0cmVhbWVyXSBQcm9ibGVtcyBwbGF5aW5nIEFBQyBzdHJlYW1zCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDIyMDQKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBQdXNoIHRoZSBFT1Mg ZXZlbnQgaW4gdGhlIFdlYktpdFdlYlNyYyBlbGVtZW50IHdoZW4gdGhlIGludGVybmFsCisgICAg ICAgIHF1ZXVlIGlzIGVtcHR5IG9ubHkgZm9yIG5vbi1saXZlIHJlc291cmNlcy4gSW4gdGhlIGxp dmUgY2FzZSB0aGUKKyAgICAgICAgcXVldWUgbWlnaHQgZHJhaW4gdGVtcG9yYXJpbHkgYnV0IHRo YXQncyBPSyBiZWNhdXNlIGRhdGEgd291bGQKKyAgICAgICAgc3RpbGwgYmUgcXVldWVkIGluIGRv d25zdHJlYW0gZWxlbWVudHMuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLCB0aGUgRU9TIGV2ZW50 IHdhcyBwdXNoZWQgdG9vIGVhcmx5IGJlY2F1c2Ugb2YgdGhpcworICAgICAgICByZWdyZXNzaW9u IGJ1dCB0aGVyZSdzIG5vIHdheSB0byBkaWZmZXJlbnRpYXRlIGl0IGZyb20gYW4gZXhwZWN0ZWQK KyAgICAgICAgZXZlbnQgaW4gYSBsYXlvdXQgdGVzdC4KKworICAgICAgICAqIHBsYXRmb3JtL2dy YXBoaWNzL2dzdHJlYW1lci9XZWJLaXRXZWJTb3VyY2VHU3RyZWFtZXIuY3BwOgorICAgICAgICAo d2ViS2l0V2ViU3JjQ3JlYXRlKToKKwogMjAxOS0xMC0wNiAgUnlvc3VrZSBOaXdhICA8cm5pd2FA d2Via2l0Lm9yZz4KIAogICAgICAgICBhdHRhY2hTaGFkb3cgc2hvdWxkIHN1cHBvcnQgYXR0YWNo aW5nIGEgc2hhZG93IHJvb3QgdG8gYSBtYWluIGVsZW1lbnQKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9XZWJLaXRXZWJTb3VyY2VHU3RyZWFt ZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL1dlYktp dFdlYlNvdXJjZUdTdHJlYW1lci5jcHAKaW5kZXggOTgxMGMyNTdiMzc3NmZmZDhkNDIxMmU5Njk0 YmU5MmIyNTk3MzUwZi4uOTY1YjZlMjc2ZDAxNDU3OTFlMWVjNDdjMDExOTFmMjgzZDRhYmM0ZSAx MDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL1dl YktpdFdlYlNvdXJjZUdTdHJlYW1lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvZ3N0cmVhbWVyL1dlYktpdFdlYlNvdXJjZUdTdHJlYW1lci5jcHAKQEAgLTQ1MSwx MSArNDUxLDExIEBAIHN0YXRpYyBHc3RGbG93UmV0dXJuIHdlYktpdFdlYlNyY0NyZWF0ZShHc3RQ dXNoU3JjKiBwdXNoU3JjLCBHc3RCdWZmZXIqKiBidWZmZXIpCiAgICAgICAgICAgICBHU1RfQlVG RkVSX09GRlNFVCgqYnVmZmVyKSA9IGJhc2VTcmMtPnNlZ21lbnQucG9zaXRpb247CiAgICAgICAg ICAgICBHU1RfQlVGRkVSX09GRlNFVF9FTkQoKmJ1ZmZlcikgPSBHU1RfQlVGRkVSX09GRlNFVCgq YnVmZmVyKSArIHNpemU7CiAgICAgICAgICAgICBHU1RfVFJBQ0VfT0JKRUNUKHNyYywgIkJ1ZmZl ciBib3VuZHMgc2V0IHRvICUiIEdfR1VJTlQ2NF9GT1JNQVQgIi0lIiBHX0dVSU5UNjRfRk9STUFU LCBHU1RfQlVGRkVSX09GRlNFVCgqYnVmZmVyKSwgR1NUX0JVRkZFUl9PRkZTRVRfRU5EKCpidWZm ZXIpKTsKLSAgICAgICAgICAgIEdTVF9UUkFDRV9PQkpFQ1Qoc3JjLCAiZG9lc0hhdmVFT1M6ICVz LCB3YXNTZWVraW5nOiAlcywgc2Vla2luZzogJXMsIHNpemU6ICV1IiwgYm9vbEZvclByaW50aW5n KHByaXYtPmRvZXNIYXZlRU9TKSwgYm9vbEZvclByaW50aW5nKHByaXYtPndhc1NlZWtpbmcpLCBi b29sRm9yUHJpbnRpbmcocHJpdi0+aXNTZWVraW5nKSwgc2l6ZSk7CisgICAgICAgICAgICBHU1Rf VFJBQ0VfT0JKRUNUKHNyYywgImRvZXNIYXZlRU9TOiAlcywgd2FzU2Vla2luZzogJXMsIHNlZWtp bmc6ICVzLCBidWZmZXIgc2l6ZTogJXUsIHNpemU6ICUiIEdfR1VJTlQ2NF9GT1JNQVQsIGJvb2xG b3JQcmludGluZyhwcml2LT5kb2VzSGF2ZUVPUyksIGJvb2xGb3JQcmludGluZyhwcml2LT53YXNT ZWVraW5nKSwgYm9vbEZvclByaW50aW5nKHByaXYtPmlzU2Vla2luZyksIHNpemUsIHByaXYtPnNp emUpOwogICAgICAgICAgICAgaWYgKHByaXYtPmhhdmVTaXplICYmIEdTVF9CVUZGRVJfT0ZGU0VU X0VORCgqYnVmZmVyKSA+PSBwcml2LT5zaXplKSB7CiAgICAgICAgICAgICAgICAgaWYgKHByaXYt Pndhc1NlZWtpbmcpCiAgICAgICAgICAgICAgICAgICAgIHByaXYtPndhc1NlZWtpbmcgPSBmYWxz ZTsKLSAgICAgICAgICAgICAgICBlbHNlCisgICAgICAgICAgICAgICAgZWxzZSBpZiAocHJpdi0+ aXNTZWVrYWJsZSkKICAgICAgICAgICAgICAgICAgICAgcHJpdi0+ZG9lc0hhdmVFT1MgPSB0cnVl OwogICAgICAgICAgICAgfSBlbHNlIGlmIChwcml2LT53YXNTZWVraW5nKQogICAgICAgICAgICAg ICAgIHByaXYtPndhc1NlZWtpbmcgPSBmYWxzZTsK