37034 2010-04-02 11:49:42 -0700 [GStreamer] Player doesn't return to NETWORK_IDLE when not using network 2021-04-14 06:36:15 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) PC Linux NEW P2 Normal --- 1 scherkus webkit-unassigned calvaris eric.carlson mrobinson pnormand sjl oldest_to_newest 207976 0 scherkus 2010-04-02 11:49:42 -0700 In order to implement the suspend event, MediaPlayerPrivate implementations should return to NETWORK_IDLE when not using the network. 254718 1 pnormand 2010-07-22 04:01:34 -0700 I don't see NETWORK_LOADED in the spec anymore while we still use it. http://www.w3.org/TR/html5/video.html#dom-media-networkstate Should we replace Loaded with Idle? 254719 2 pnormand 2010-07-22 04:02:51 -0700 BTW the test to fix/unskip is http/tests/media/video-play-suspend.html 254736 3 pnormand 2010-07-22 05:10:13 -0700 LOADED is used when a media is fully buffered I think and IDLE is when network is not used, so I guess it's kind of not the same ;) I'm still confused though to still see we use LOADED and the spec not mentionning it anymore, or I'm reading the wrong one. 254777 4 eric.carlson 2010-07-22 07:31:00 -0700 (In reply to comment #1) > I don't see NETWORK_LOADED in the spec anymore while we still use it. > > http://www.w3.org/TR/html5/video.html#dom-media-networkstate > NETWORK_LOADING and the 'loaded' event went away in r63684 (and a few follow-on build fixes ;-). (In reply to comment #3) > LOADED is used when a media is fully buffered I think > and IDLE is when network is not used, so I guess it's kind of not the same ;) > > I'm still confused though to still see we use LOADED and the spec not mentionning it > anymore, or I'm reading the wrong one. We still have the internal state MediaPlayer::Loaded, but that translates to networkState == NETWORK_IDLE. This internal state is necessary so we can tell if we need to fire the 'suspend' event or not. The spec says this about the 'suspend' event: The user agent is intentionally not currently fetching media data, but does not have the entire media resource downloaded. http://www.w3.org/TR/html5/video.html#mediaevents The 'suspend' event is only fired when the media engine sets networkState to MediaPlayer::Idle itself. 255337 5 pnormand 2010-07-23 05:53:46 -0700 (In reply to comment #4) > (In reply to comment #3) > > LOADED is used when a media is fully buffered I think > > and IDLE is when network is not used, so I guess it's kind of not the same ;) > > > > I'm still confused though to still see we use LOADED and the spec not mentionning it > > anymore, or I'm reading the wrong one. > We still have the internal state MediaPlayer::Loaded, but that translates to networkState == NETWORK_IDLE. This internal state is necessary so we can tell if we need to fire the 'suspend' event or not. The spec says this about the 'suspend' event: > > The user agent is intentionally not currently fetching media data, but does not have the entire media resource downloaded. > > http://www.w3.org/TR/html5/video.html#mediaevents > > The 'suspend' event is only fired when the media engine sets networkState to MediaPlayer::Idle itself. Hum does that mean we should set networkState to Idle when the media is fully loaded? Sorry but I don't see how to make that test pass without doing that, simply because the media is local, download is instantaneous, so I don't see why a suspend event should be fired in this case. In progressive download mode GStreamer will keep the download running even if the playback is paused, until it fully completes, AFAIK. 360129 6 sjl 2011-03-01 12:45:10 -0800 FYI, http/tests/media/video-play-suspend.html has been removed as the test isn't actually valid according to spec. See https://bugs.webkit.org/show_bug.cgi?id=55198 for details. 360639 7 pnormand 2011-03-02 00:31:38 -0800 Thanks for the heads up Steve! Seems like this new test is passing on GTK. Does this mean we can close this bug? 361017 8 sjl 2011-03-02 13:05:58 -0800 You need to make sure you set the network state to loaded before transitioning to the final idle so that a final progress event is fired (see HTMLMediaElement.cpp). If you're doing this, all should be good. 367036 9 85687 pnormand 2011-03-14 10:05:47 -0700 Created attachment 85687 proposed patch 367038 10 85687 mrobinson 2011-03-14 10:06:57 -0700 Comment on attachment 85687 proposed patch Are there any tests covering this? 367044 11 pnormand 2011-03-14 10:23:46 -0700 (In reply to comment #10) > (From update of attachment 85687 [details]) > Are there any tests covering this? Hum it doesn't seem so. I'll try to write one :) Will clear the r+ flag for now. 828669 12 mrobinson 2013-02-08 10:35:45 -0800 If writing a test is a lot of work, it's probably okay just to land this as-is. 1750090 13 calvaris 2021-04-14 06:36:15 -0700 There is a http/tests/media/video-load-suspend.html test and it seems to check out now. I think we can close this. 85687 2011-03-14 10:05:47 -0700 2011-03-14 10:23:59 -0700 proposed patch proposed-patch.patch text/plain 2299 pnormand RnJvbSAxMTkzMmU0ZWY1MWFhOGFhNGJmZDE5MWZkZTcwMGVlYjM2ZTJmYzM3IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQaGlsaXBwZSBOb3JtYW5kIDxwbm9ybWFuZEBpZ2FsaWEuY29t PgpEYXRlOiBNb24sIDE0IE1hciAyMDExIDE4OjA3OjA0ICswMTAwClN1YmplY3Q6IFtQQVRDSF0g cHJvcG9zZWQgcGF0Y2gKCi0tLQogU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nICAgICAgICAgICAg ICAgICAgICAgICAgICAgfCAgIDE0ICsrKysrKysrKysrKysrCiAuLi4vZ3N0cmVhbWVyL01lZGlh UGxheWVyUHJpdmF0ZUdTdHJlYW1lci5jcHAgICAgICB8ICAgIDggKysrKysrKysKIDIgZmlsZXMg Y2hhbmdlZCwgMjIgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg YzgxNzA1MS4uY2ZkMWQwYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisr KyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDExLTAzLTE0 ICBQaGlsaXBwZSBOb3JtYW5kICA8cG5vcm1hbmRAaWdhbGlhLmNvbT4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNZWRpYVBsYXllclByaXZhdGVHU3Ry ZWFtZXIgZG9lc24ndCByZXR1cm4gdG8gTkVUV09SS19JRExFIHdoZW4gbm90IHVzaW5nIG5ldHdv cmsKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM3MDM0 CisKKyAgICAgICAgQWZ0ZXIgb24tZGlzayBidWZmZXJpbmcgY29tcGxldGVzIGFsbG93IHRoZSBt ZWRpYVBsYXllckNsaWVudCB0bworICAgICAgICBmaXJlIGEgc3VzcGVuZCBldmVudCBieSBzd2l0 Y2hpbmcgdG8gTE9BREVEIGJlZm9yZSBnb2luZyB0byB0aGUKKyAgICAgICAgSURMRSBuZXR3b3Jr IHN0YXRlLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxh eWVyUHJpdmF0ZUdTdHJlYW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpNZWRpYVBsYXllclBy aXZhdGVHU3RyZWFtZXI6OnVwZGF0ZVN0YXRlcyk6CisKIDIwMTEtMDMtMTQgIE1pa2hhaWwgTmFn YW5vdiAgPG1uYWdhbm92QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBQYXZl bCBGZWxkbWFuLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mv Z3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5jcHAgYi9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVy LmNwcAppbmRleCAzNzYzZWY3Li5iM2NlMDA5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNw cAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQ bGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcApAQCAtOTc4LDYgKzk3OCwxNCBAQCB2b2lkIE1lZGlh UGxheWVyUHJpdmF0ZUdTdHJlYW1lcjo6dXBkYXRlU3RhdGVzKCkKICAgICAgICAgICAgIGNhY2hl RHVyYXRpb24oKTsKICAgICAgICAgfSBlbHNlIGlmIChtYXhUaW1lTG9hZGVkKCkgPT0gZHVyYXRp b24oKSkgewogICAgICAgICAgICAgbV9uZXR3b3JrU3RhdGUgPSBNZWRpYVBsYXllcjo6TG9hZGVk OworICAgICAgICAgICAgaWYgKG9sZE5ldHdvcmtTdGF0ZSAhPSBtX25ldHdvcmtTdGF0ZSkKKyAg ICAgICAgICAgICAgICBtX3BsYXllci0+bmV0d29ya1N0YXRlQ2hhbmdlZCgpOworCisgICAgICAg ICAgICAvLyBJZiBvbi1kaXNrIGJ1ZmZlcmluZyBmaW5pc2hlZCB0aGUgbmV0d29yayBzaGFsbCBu b3QgYmUKKyAgICAgICAgICAgIC8vIHVzZWQgYW55bW9yZSB0byBhY2Nlc3MgdGhlIG1lZGlhLgor ICAgICAgICAgICAgaWYgKCFtX2ZpbGxUaW1lci5pc0FjdGl2ZSgpKQorICAgICAgICAgICAgICAg IG1fbmV0d29ya1N0YXRlID0gTWVkaWFQbGF5ZXI6OklkbGU7CisKICAgICAgICAgICAgIG1fcmVh ZHlTdGF0ZSA9IE1lZGlhUGxheWVyOjpIYXZlRW5vdWdoRGF0YTsKICAgICAgICAgfSBlbHNlIHsK ICAgICAgICAgICAgIG1fcmVhZHlTdGF0ZSA9IGN1cnJlbnRUaW1lKCkgPCBtYXhUaW1lTG9hZGVk KCkgPyBNZWRpYVBsYXllcjo6SGF2ZUZ1dHVyZURhdGEgOiBNZWRpYVBsYXllcjo6SGF2ZUN1cnJl bnREYXRhOwotLSAKMS43LjQuMQ==