Skip to content

LLAMA-7406: Delay playback pos query after seek#938

Closed
Scony wants to merge 2 commits into
WebPlatformForEmbedded:wpe-2.28from
Scony:delay-playback-pos-query-after-seek
Closed

LLAMA-7406: Delay playback pos query after seek#938
Scony wants to merge 2 commits into
WebPlatformForEmbedded:wpe-2.28from
Scony:delay-playback-pos-query-after-seek

Conversation

@Scony
Copy link
Copy Markdown

@Scony Scony commented Sep 12, 2022

Change on top of #926

@woutermeek woutermeek requested a review from eocanha September 15, 2022 07:37
@emutavchi emutavchi added the wpe-2.28 Only for PR affecting 2.28 label Sep 20, 2022
@eocanha
Copy link
Copy Markdown
Member

eocanha commented Sep 21, 2022

Please, comment on my question on #926 (comment) so that I can continue with #926 and then with this PR.

@eocanha
Copy link
Copy Markdown
Member

eocanha commented Sep 22, 2022

Likely this PR (commit 44eed71) won't be upstreamed, since the code it touches has been heavily refactored upstream. Still, it depends on the final version of #926 landing upstream as WebKit/WebKit#4601

webkit-early-warning-system pushed a commit to eocanha/WebKit that referenced this pull request Sep 27, 2022
https://bugs.webkit.org/show_bug.cgi?id=245529

Reviewed by Xabier Rodriguez-Calvar.

Some devices have audio sinks that don't not support async state changes i.e. it completes
transition to PAUSED state on preroll. In those cases, there's a need to be able to finish
seeks in sync state changes (currently, only seeks on async state changes are supported).

This patch adds support to handle that use case by finishing the seek from updateStates().
This new code makes the old processing in asyncStateChangeDone() redundant, because in those
circumstances the handling of the GST_MESSAGE_STATE_CHANGED message in handleMessage()
already calls updateStates(). Still, asyncStateChangeDone() has been preserved because it's
virtual and the MediaPlayerPrivateGStreamerMSE subclass still needs it for its own purpose.

The code to finish seek has been refactored to its own function. This isn't strictly needed
but would be really helpful for our downstream port, where
WebPlatformForEmbedded/WPEWebKit#938 would benefit from it.

Original author: Pawel Lampe <pawel.lampe@gmail.com>
See: WebPlatformForEmbedded/WPEWebKit#926

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::handleMessage): Added comment about super and subclass processing needs for GST_MESSAGE_ASYNC_DONE.
(WebCore::MediaPlayerPrivateGStreamer::finishSeek): Refactored code to finish seek.
(WebCore::MediaPlayerPrivateGStreamer::updateStates): Process finish seek from here.
(WebCore::MediaPlayerPrivateGStreamer::asyncStateChangeDone): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
(WebCore::MediaPlayerPrivateGStreamer::asyncStateChangeDone): Now empty implementation for the superclass.

Canonical link: https://commits.webkit.org/254923@main
eocanha added a commit that referenced this pull request Sep 28, 2022
https://bugs.webkit.org/show_bug.cgi?id=245529

Reviewed by Xabier Rodriguez-Calvar.

Some devices have audio sinks that don't not support async state changes i.e. it completes
transition to PAUSED state on preroll. In those cases, there's a need to be able to finish
seeks in sync state changes (currently, only seeks on async state changes are supported).

This patch adds support to handle that use case by finishing the seek from updateStates().
This new code makes the old processing in asyncStateChangeDone() redundant, because in those
circumstances the handling of the GST_MESSAGE_STATE_CHANGED message in handleMessage()
already calls updateStates(). Still, asyncStateChangeDone() has been preserved because it's
virtual and the MediaPlayerPrivateGStreamerMSE subclass still needs it for its own purpose.

The code to finish seek has been refactored to its own function. This isn't strictly needed
but would be really helpful for our downstream port, where
#938 would benefit from it.

Original author: Pawel Lampe <pawel.lampe@gmail.com>
See: #926

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::handleMessage): Added comment about super and subclass processing needs for GST_MESSAGE_ASYNC_DONE.
(WebCore::MediaPlayerPrivateGStreamer::finishSeek): Refactored code to finish seek.
(WebCore::MediaPlayerPrivateGStreamer::updateStates): Process finish seek from here.
(WebCore::MediaPlayerPrivateGStreamer::asyncStateChangeDone): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
(WebCore::MediaPlayerPrivateGStreamer::asyncStateChangeDone): Now empty implementation for the superclass.

Canonical link: https://commits.webkit.org/254923@main
@eocanha
Copy link
Copy Markdown
Member

eocanha commented Sep 28, 2022

I've rebased the commit on top of the final version of #926 and pushed the remaining commit in this PR to wpe-2.28 as 7f14132, closing PR.

@eocanha eocanha closed this Sep 28, 2022
aperezdc pushed a commit to WebKit/WebKit that referenced this pull request Sep 29, 2022
…ange

https://bugs.webkit.org/show_bug.cgi?id=245529

Reviewed by Xabier Rodriguez-Calvar.

Some devices have audio sinks that don't not support async state changes i.e. it completes
transition to PAUSED state on preroll. In those cases, there's a need to be able to finish
seeks in sync state changes (currently, only seeks on async state changes are supported).

This patch adds support to handle that use case by finishing the seek from updateStates().
This new code makes the old processing in asyncStateChangeDone() redundant, because in those
circumstances the handling of the GST_MESSAGE_STATE_CHANGED message in handleMessage()
already calls updateStates(). Still, asyncStateChangeDone() has been preserved because it's
virtual and the MediaPlayerPrivateGStreamerMSE subclass still needs it for its own purpose.

The code to finish seek has been refactored to its own function. This isn't strictly needed
but would be really helpful for our downstream port, where
WebPlatformForEmbedded/WPEWebKit#938 would benefit from it.

Original author: Pawel Lampe <pawel.lampe@gmail.com>
See: WebPlatformForEmbedded/WPEWebKit#926

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::handleMessage): Added comment about super and subclass processing needs for GST_MESSAGE_ASYNC_DONE.
(WebCore::MediaPlayerPrivateGStreamer::finishSeek): Refactored code to finish seek.
(WebCore::MediaPlayerPrivateGStreamer::updateStates): Process finish seek from here.
(WebCore::MediaPlayerPrivateGStreamer::asyncStateChangeDone): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
(WebCore::MediaPlayerPrivateGStreamer::asyncStateChangeDone): Now empty implementation for the superclass.

Canonical link: https://commits.webkit.org/254923@main

(cherry picked from commit a8ae25e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wpe-2.28 Only for PR affecting 2.28

Development

Successfully merging this pull request may close these issues.

3 participants