WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
211 stars 135 forks source link

With WPEWebkit 2.28, disposing player is not called #1229

Closed modeveci closed 6 months ago

modeveci commented 9 months ago

When using WPEWebit 2.22, when in the CNN app playing a video, if you press the HOME key on the RCU, then the mediaplayer is properly deleted, and MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer() is called. This leaves Gstreamer, and all the components involved in the pipeline, in a correct state. However, when using WPEWebkit 2.28 instead, when doing the same test the mediaplayer is NOT properly deleted, and MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer() is NOT called. This leaves some of the pipeline elements in a wrong state, and triggers later playback issues on some platforms. Why isn't the mediaplayer properly destroyed? Is it a design decision, or an implementation issue? How to re-enable the proper destruction of the mediaplayer when an app is destroyed?

magomez commented 9 months ago

@modeveci, can you provide the URL of that CNN app, please?

modeveci commented 9 months ago

@magomez It states CNN app, but it should be observable with any playback during deactivation.

pgorszkowski-igalia commented 8 months ago

@modeveci : by "press the HOME key on the RCU" you mean that, in consequence, you send to wpe request to switch to "boot" site, right? I have done such test on RPi and WPE 2.28 and it seems to work as expected. Can you provide logs from your case? GST_DEBUG="webkitmediaplayer:5" and WEBKIT_DEBUG=Media,Network

modeveci commented 8 months ago

Pressing Home means to Deactivate from WPEFramework UI, so it is being controlled by WPEBrowser Plugin. The point is when WebPage/WebView etc. are destructed when there is an active playback, the disposal of player is not observed bt them. I am sharing some logs. wpeframework.log

pgorszkowski-igalia commented 8 months ago

Ok, now I am able to reproduce the problem on RPi and WPE 2.28.

modeveci commented 7 months ago

Hi @pgorszkowski-igalia , Is this PR: https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1271 about this issue?

pgorszkowski-igalia commented 7 months ago

It seems that fix from https://bugs.webkit.org/show_bug.cgi?id=217655 solves the problem.

The main problem here was that message about closing from UI to WebProcess is not sent because right after "close" message is set in queue to send from UI to Web process , the connection is closed. What worst, when the connection is closed then WebProcess was destroyed (by calling _exit) without any cleanups. With change from https://github.com/WebKit/WebKit/commit/28d9d62aa2662266c2d9cb0032b036a257c6c8ab WebProcess will close (and clean ups all resources) all pages before it exits.

The backport PR is: https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1271

pgorszkowski-igalia commented 7 months ago

Hi @pgorszkowski-igalia , Is this PR: #1271 about this issue?

Yes, I thought that my previous comment was published but for unknown reason, it was not, sorry for that.

pgorszkowski-igalia commented 7 months ago

@modeveci: https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1271 is merged, have you tested it? can we close this ticket?

modeveci commented 7 months ago

@pgorszkowski-igalia i have asked the reporter to test, as soon as I have received an update, i will close this ticket. Thank you very much!

modeveci commented 7 months ago

77862_patch_log1.txt It seems the problem is still present. can you look at the attached logs @pgorszkowski-igalia ? thank you!

pgorszkowski-igalia commented 7 months ago

I think it works, Disposing player log is seen after Shutdown: Deactivated plugin [WebKitBrowser]:[Lightning]

Jan 23 05:57:09.692205 /usr/bin/WPEFramework[5737]: [Tue, 23 Jan 2024 05:57:09 ]:[PluginServer.cpp:487]: Shutdown: Deactivated plugin [WebKitBrowser]:[Lightning]
...
Jan 23 05:57:09.703543 WPEFramework[7745]: 0:00:16.618196105  7745   0x7f88004500 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:266:~MediaPlayerPrivateGStreamer:<media-player-0> Disposing player
modeveci commented 7 months ago

@pgorszkowski-igalia I see, I am asking them further details and stating your feedback. Thank you!

pgorszkowski-igalia commented 6 months ago

@modeveci : do we have a feedback here? can we close it?

modeveci commented 6 months ago

@pgorszkowski-igalia I am waiting for the reporter confirmation. No update so far!

modeveci commented 6 months ago

@pgorszkowski-igalia it has been reported as resolved. Thank you very much!