Refactor event-patterns, use MediaSessions API#69
Merged
rtshkmr merged 50 commits intofeature/hanumanfrom Aug 10, 2024
Merged
Conversation
Member
Author
|
Here's a migrator json dump that can be used. Contains the two videos: will generate another one once the content of the metadata is clear |
note: because of this, the PR requires the deployment to use a fresh json file this prepares for the info needed by media sessions does some linting using the latest elixir-ls
Member
Author
|
Contains some metadata about the two voice files we have |
Not sure why it doesn't display when using the emulator. I think it's better if I test it on the deployed version directly, not sure if the testing method is accurate.
ks0m1c
reviewed
Jul 31, 2024
Contributor
|
388c966 to
f15ef26
Compare
Init functions don't have action handlers yet
mainly consolidates the functions used for updating playback and notifying the audio player
Separates the concerns b/w the MediaBridge (generic) and the AudioPlayer (concrete)
Change list:
1. reorganise bridges into hooks/mediaEventBridges, within it there's a
new playbackMetaBridge. Intent is to comms playback info and separate
actions from playback info things.
2. now, when the server does a push_event for the audio events
registration, we also send a separate event that is using this new
bridge ("media_bridge:registerPlayback"), which dispatches the necessary
client side events to register the playback, load the audio and the
mediaSession as early as possible. Previously, this used to happen JIT,
when the user was to press the play/pause button. This seems to have
removed the initial time-delay we used to have between the instance when
user clicks "play" and actual playback starting.
3. naturally, we also prevent redundant re-loads of audio now by
guarding the source-setting for the html5 audio player.
Broad TODOs:
1. consider pushing to the new playbackMetaBridge in the same intervals
as the existing heartbeat.
2. this commit only added in message-passing from MediaBridgeHook to the
AudioPlayerHook, but not in the other direction.
Playback state is init @ point of events registration, via a new bridge now.
Automagically works == pat on the back for good design?
ks0m1c
reviewed
Aug 10, 2024
Contributor
ks0m1c
left a comment
There was a problem hiding this comment.
hmm not much changes here just a lot of linting looks g
| * Ideally generic hook for floating logic. | ||
| */ | ||
| import {isMobileDevice} from "../utils/uncategorised_utils.js"; | ||
| import { |
Contributor
There was a problem hiding this comment.
post vendor floater import g
| def render(assigns) do | ||
| ~H""" | ||
| <div id="audio-player" phx-hook="AudioPlayer"> | ||
| <audio data-playback={Jason.encode!(@playback)}></audio> |
Contributor
There was a problem hiding this comment.
ah the source of the data-playback change
Refactor the event-handling patterns
# Table of Contents
1. [Refactor to separate concerns MediaBridge vs others](#org25f2ca5)
1. [Overview](#orged49cc5)
2. [Initial Context & Reason to refactor:](#org8692118)
3. [More Backlogged Tasks](#org0aa6c21)
1. [handshake comms failure – init handshake @ playback time as a fallback](#orgf5a1bcc)
2. [MediaSession: handle phone lock screen, which appears to kill the websocket](#orgb0027c7)
3. [MediaSession: use the playbackMetaBrige to push metadata updates at the same interval as the existing heartbeats.](#org9f8c929)
4. [MediaSession: add event handlers for scribing / seekTo / fastSeek](#orgfd02646)
5. [MediaBridge –> MediaLibrary + voice fetching to happen @ MediaBridge.](#org4f37b9e)
4. [Some Implementaion Notes:](#org48a16ee)
<a id="org25f2ca5"></a>
# Refactor to separate concerns MediaBridge vs others
<a id="orged49cc5"></a>
## Overview
This PR cleans up the current patterns on how events flow through the server-side live components and the event bridge pattern that we’ve used. Additionally, the following things have also been done:
1. Improve ACK-ing of handshakes b/w the MediaBridge and the Written contexts. Duplicate ACKs are now ignored, thereby preventing redundant triggers of client-side hooks.
2. A new client side event bridge got added (`playbackMetaBrige`) which allows message passing just for playback metadata via that bridge. This bridge is now being used to load the audio at the earlier possible time. In particular, this is the time at which the written context is loaded, which pubs to the MediaBridge ([ref commit](d627c53)).
3. Fix the AudioPlayer hook crashing ([ref commit](26a1ae9)):
- the actual mechanics of this wasn’t deeply investigated because it initially presented as a flaky behaviour and primarily affecting Chrome browsers.
- the current hypothesis is that it’s because props passing via the `audio_player.ex` was not a good idea since when the playback state gets updated, that component will remount and create unnecessary chaos.
- this got fixed as a side-effect of registering playback preemptively.
4. Adds action handlers for play pause events that are triggered externally, via the mediasessions api.
<a id="org8692118"></a>
## Initial Context & Reason to refactor:
We made the media bridge to coordinate the playback of mediums and allow coordination
Its intent was to supposed to be a Bridge pattern which separates the abstraction and the implementation and be a bridge to the implementation.
Let’s standardise some terms – for coordinating events, we can classify the type of events into:
1. [USER EVENTS] events due to user interaction directly on the webapp
2. [EXT EVENTS] events due to external interactions (e.g. mediaSessions API receives an event trigger from the user-agent)
In an almost CQRS-fashion, we should separate the event emission and consumption such that:
- USER EVENTS flow in a single direction, from the server side live component to the MediaBridge, followed by the MediaBridge hook which then uses the EventBridges to dispatch messages to listeners on the event bridge.
- the MediaBridge hook is the broker for events on the client-side. No one shall by-pass the broker (e.g. by doing an event handling directly by one of the other Hooks, although that’s absolutely feasible since events on the client-side can be captured by any DOM node since they are dispatched at the scope of the document.)
- EXT EVENTS flow in a single direction as well wherein they emit messages to the server-side, upon which the event handling follows the same pattern as a USER EVENT and the flow of events follows the same standardised direction.
By doing this refactor, the events handling will be cleaner.
- currently we don’t have that behaviour,
- we have some duplication of efforts and weird flows because e.g. :
- `audio_player.ex` [sends](file:///Users/rtshkmr/Projects/vyasa/lib/vyasa_web/components/audio_player.ex) the playback info directly to the audio player hook so that it can read the playback info (e.g. in the handlePlayableState)
- the playback state send via the playPause bridge is the exact same source of info that gets read by the [handleMediaPlayPause](file:///Users/rtshkmr/Projects/vyasa/assets/js/hooks/audio_player.js)
- here’s the proposed FIX:
- User actions should flow through the bridge first and then go to other hooks
<a id="org0aa6c21"></a>
## More Backlogged Tasks
<a id="orgf5a1bcc"></a>
### [ ] handshake comms failure – init handshake @ playback time as a fallback
Basically, what happens when the current handshake methodology fails? When to re-init it?
<a id="orgb0027c7"></a>
### - [ ] MediaSession: handle phone lock screen, which appears to kill the websocket
The current pattern is that play-pause events are to be captured by the AudioPlayer hook and then emitted to the MediaBridge server livecomponent which handles similarly to a user event. This breaks down in the case where a user locks their phone and the websocket is dead / inactive in the background.
A pedestrian solution would be to @ heartbeats, let there be a listener module which handles:
- listening and log-stream creation
- possible: teardown of a hook – save last known states to localStorage and re-init when user’s screen is unlocked again. This could be part of an overall push for better network-robustness as well.
<a id="org9f8c929"></a>
### [ ] MediaSession: use the playbackMetaBrige to push metadata updates at the same interval as the existing heartbeats.
<a id="orgfd02646"></a>
### [ ] MediaSession: add event handlers for scribing / seekTo / fastSeek
So that the media session API, which can already be used to do those operations on the playback, will be synced to states that we are managing on our live components.
<a id="org4f37b9e"></a>
### [ ] MediaBridge –> MediaLibrary + voice fetching to happen @ MediaBridge.
This means that others may request to play voices based on voice ID, but the DB transacts shall be on the MediaLibrary side. This better aligns with the design seen in the LiveBeats repo as well.
<a id="org48a16ee"></a>
## Some Implementaion Notes:
1. the initSession() for the audio player hook won’t work anymore, we want to rm the playback payload that is passed
2. ACK:
- the audio player has some bufferring to do. We want the player to send a broadcast to the others (esp. the MediaBridge hook) that it’s ready to play.
2 ways to do this:
1. [mediated by the server] audio player hook ==> audio player server live component ==> media bridge live component ==> media bridge hook
2. [no server mediation] audio player hook ==> media bridge hook
- my champion solution is b). For now we don’t need to support group singing yet, so client-side broadcast of buffer-state should be sufficient.
3. audio player ready state event.
- using the `canplaythrough` event that gets emitted by the player for this. It dispatches when the player thinks that playback can happen from start to end without bufferring. ([ref](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/canplaythrough_event))
4. livebeats has a separate hook just for pings ([ref](file:///Users/rtshkmr/Projects/live_beats/assets/js/app.js)). This acts as the heartbeat. It seems that it’s only being used to broadcast active users that are currently tuned in ([ref broadcast fn](file:///Users/rtshkmr/Projects/live_beats/lib/live_beats_web/live/nav.ex)).
5. [difference] livebeats gets its song via song id from the [medialibrary](file:///Users/rtshkmr/Projects/live_beats/lib/live_beats/media_library.ex) (our media bridge), in our case, we’re getting it from the Medium, called by the [chapter::index.ex](file:///Users/rtshkmr/Projects/vyasa/lib/vyasa_web/live/source_live/chapter/index.ex)
rtshkmr
commented
Aug 10, 2024
| def get_event_by_order!(%Event{origin: origin, voice_id: v_id}, order) do | ||
| #TODO merge Sangh Filters to fix -1 order case for origin backwards operators | ||
| (from e in Event, | ||
| # TODO merge Sangh Filters to fix -1 order case for origin backwards operators |
Contributor
There was a problem hiding this comment.
this was actually built out for the admin view idea before Repo.Paginated.all(opts) was merged. with Vyasa.Repo.Paginated module filter is expressed as
query
|> Repo.Paginated.all([filter: origin, sort_attribute: :origin, limit: 1])
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR does a bunch of things:
Migration Notes:
1723306013568018000.json