-
Notifications
You must be signed in to change notification settings - Fork 152
[2.46][GST][MSE] Skip playback state update while EOS #1568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: wpe-2.46
Are you sure you want to change the base?
[2.46][GST][MSE] Skip playback state update while EOS #1568
Conversation
Playback state change notification syncs Media Client (HTMLMediaElement) state with current player state: commit fb0730e: The platform backend may change state, for example as a result of an external plugin controlling the backend, so we need to react to this situation by syncing up the WebCore state with the platform backend. We call playInternal()/pauseInternal() depending on the backend state, to trigger the corresponding DOM events to match the state. Sending that notification on EOS condition may effectively pause the pipeline without user request becasue the GST player reports it is paused m_isEndReached is set.
Here is a test case for that https://asurdej-comcast.github.io/tr/tests/RDKTV-38576_mse_seek_at_eos.html The flow is:
With current impl the playback gets stuck in paused state and never restarts |
Here is a sequence of events from WebKit perspective that leads to video stall:
My proposition fixes the bold bullet, While m_isEndReached is set, it doesn't send playbackStateChanged() notification to HTMLMediaElement so it will never accept the paused state and won't force it later on MediaPlayer |
I'm having a look at this next Monday Oct 6th. |
I'm still trying to understand all the steps, but I wonder (to myself) if, instead of ignoring the m_isEndReached flag as a sort of hacky workaround, there's any way of resetting the flag ahead of time (even before the seek starts to be processed asynchronously), so that it's false already at the right time. I'm trying to analyze if this is possible, for instance by using willSeekToTarget(), which sets m_pendingSeekTime. I still haven't checked if that call happens soon enough, tho. I'll continue analyzing it. |
I'm having a hard time to get your test case to work properly (at least on a Raspberry Pi, wpe-2.46), even after having mirrored it on a local server and added extra logs. First, I needed to increase the timeouts (the data load was slow for some reason). Then, I'm having spureous "can't push buffer because the pad is on EOS" failures in one of the WebKitMediaSrc pads. All this applying only your suggested fix, instead of mine. I think I only saw it working (green) one time after multiple tries. I'm still debugging what happens. |
@eocanha Thanks for looking into this. Disney plus uses the same MediaSource object and the same SourceBuffers to play both main content and advertisments. That is a bit tricky as MediaSource isn't accepting more data after endOfStream() so it has to be reopened somehow, I use timestampOffset of SourceBuffer that reopens MediaSouce. Resetting m_isEndReached, e.g. in MediaPlayerPrivate::play() would help but the GST is still at eos position so there are no more data to play so it will be reported by the sinks again. Another "problem" could be the ready state, that inside MediaSource it is calculated based on the new position (current position is taken from m_pendingSeekTime set from willSeekToTarget()) and propagated to player while the player is still at the old position |
Playback state change notification syncs Media Client (HTMLMediaElement) state with current player state:
commit fb0730e:
Sending that notification on EOS condition may effectively pause the pipeline without user request becasue the GST player reports it is paused m_isEndReached is set.
PS. We already had a bunch of problems with that playback state change notification and the way of how HTMLMediaElement handles mediaPlayerPlaybackStateChanged(). In 2.46 that part was added because of sleep disable while it was completely missing in 2.38. Maybe we should handle sleep disable separately and skip that mediaPlayerPlaybackStateChanged() at all
46455f0