[Test required] MSE::updateStates() should not wait for state change to finish

[calvaris]

While investigating https://bugs.webkit.org/show_bug.cgi?id=259775 I tried to remove the requirement of not having an ongoing state change to be able to perform another state change. I tried upstream and then downstream with the reference box and rpi. It seems to work but I fear it might impact any custom platform I don't have access to.

I would require testing to ensure this is not a problem. Then I would port it upstream and follow the procedure of upstreaming and then bringing back.
3540c5d

Build-Tests	Layout-Tests
✅ 🛠 wpe-238-amd64-build	✅ 🧪 wpe-238-amd64-layout
✅ 🛠 wpe-238-arm32-build	✅ 🧪 wpe-238-arm32-layout

[asurdej-comcast]

I had similar change in the past (to wait for seek finish) #1132 (review)
it was needed for some mixture of play, pause, seek and rate change but not sure if this is needed still. There is a test case attached, I will try it tomorrow

[calvaris]

I had similar change in the past (to wait for seek finish) #1132 (review) it was needed for some mixture of play, pause, seek and rate change but not sure if this is needed still. There is a test case attached, I will try it tomorrow

Yes, @asurdej-comcast , I tried it and it worked as expected, but maybe not in the platforms I don't have access to.

[asurdej-comcast]

@calvaris That was platform independent, I saw it with PC x86 webkit build also
I tried my tests and they all look good with your change here.

The case was that calling changePipelineState(GST_STATE_PAUSED); while seeking was getting rejected as pipeline was in PAUSED->PAUSED async transition. It was not reflected in m_isPipelinePlaying that was modified immediately anyway.

with c2a7752
m_isPipelinePlaying is flipped in changePipelineState(), only if state change is completed so the problem doesn't happen any more. Right now, getting rejected inside changePipelineState() is pretty much the same as not calling it at all

[calvaris]

@asurdej-comcast

The same check is inside changePipelineState() and there is no logic in updateStates() that may be affected. So we should be good in the context of play/pause.

So did you also tested removing that condition from changePipelineState?

[suresh-khurdiya-infosys]

@calvaris We have taken your PR in our code base and done some regression testing. So far, we don't see any regression.

[asurdej-comcast]

So did you also tested removing that condition from changePipelineState?

Yes, I tested with isSeeking removed

[calvaris]

@emutavchi @asurdej-comcast @suresh-khurdiya-infosys I just pushed some more bits of code. Can you please test them? I also filed WebKit/WebKit#40620 so if this works we land it everywhere.

[asurdej-comcast]

@calvaris The change is more serious now, I remember I saw yet another problem in the past, related to pausing GST pipeline during async state change. I run tests again and still everything works as expected from user perspective but not really sure if everything is fine inside GST itself.
So normally when you change pipeline state into paused (with gst_element_set_state()) all internal elements are put into paused state (on the .dot graph every element is "="). But when you do the same in async paused->paused transition only sinks are paused really ("=" on graph) and all the rest (parse/decodebin) is left in playing state still (">" on graph). The same is visible in logs. See attached pipeline graph below. I'm not really sure if that's a problem on not. This is the final state after preload is completed and async transition is finished

Full graph .dot file
0.00.03.270077490-MSE-media-player-0_PAUSED_PAUSED.zip

[calvaris]

@asurdej-comcast . Which use case is this?

[asurdej-comcast]

MSE, call video.pause() while waiting for preroll after seek https://asurdej-comcast.github.io/tests/RDK-41921_mse_seek_pause2.html
Test passes (video position is not progressing after seek) but the gst pipeline is in inconsistent state at the end.
The same result with my device gst 1.18.5 and PC flatpack build gst 1.20

[suresh-khurdiya-infosys]

@calvaris I will wait for final changes for regression testing on our platform.

[calvaris]

MSE, call video.pause() while waiting for preroll after seek https://asurdej-comcast.github.io/tests/RDK-41921_mse_seek_pause2.html Test passes (video position is not progressing after seek) but the gst pipeline is in inconsistent state at the end. The same result with my device gst 1.18.5 and PC flatpack build gst 1.20

Given this issues, let's close this.