Skip to content
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

Support playback seeking by clicking on the verse #47

Merged
merged 1 commit into from
Mar 3, 2024

Conversation

rtshkmr
Copy link
Member

@rtshkmr rtshkmr commented Feb 26, 2024

This commit finally made me feel that I understood the lang & the framework to a working-degree of familiarity.
Really happy with myself.

The seeking by clicking works well !!!

Got to click the 1.1 and it will seek!! The MP3 - timestamping - sync oddity still exists but less so for the hanuman chalisa voice

image

This commit finally made me feel that I understood the lang & the
framework to a working-degree of familiarity. Really happy with myself.

The seeking by clicking works well !!!
@rtshkmr rtshkmr self-assigned this Feb 26, 2024
@rtshkmr rtshkmr mentioned this pull request Feb 26, 2024
50 tasks
@rtshkmr
Copy link
Member Author

rtshkmr commented Feb 26, 2024

Updates on the Singleton Progress Timer

I agree that there's a value in shifting the timer to a singleton class so that it can be tied to session state and updated.
I completed a first pass of it, but it resulted in too many recursive calls.

reason for having too many recursive calls

Since the timer needs to receive a callback, and the callback that is passed to it requires the current audio player's state, I'd need to pass that callback function like so,

     }
-    this.progressTimer = setInterval(() => this.updateProgress(), progressUpdateInterval)
+    this.playbackTimer.syncProgressTimer(() => this.updateProgress())
   },

Here's the class for reference

/**
 * Playback timer shall be a singleton that playback state can make reference to.
 * The purpose of using a distinct class for this is so that timer state can be serialsed
 * and stored in session state, thereby supporting session-continuity behaviour.
 * */

export class PlaybackTimer {
  constructor() {
    if (!PlaybackTimer._instance) {
      PlaybackTimer._instance = this;
      this.progressUpdateInterval = 100 // 10fps, comfortable for human eye
      this.progressTimer = null;
    }
    return PlaybackTimer._instance;
  }

  /**
   * Calls the update_fn at a set interval,
   * replaces an existing progress timer, if it exists.
   * */
  syncProgressTimer(update_fn) {
    const hasExistingTimer = this.progressTimer
    if(hasExistingTimer) {
      this.clearProgressTimer()
    }
    this.progressTimer = setInterval(() => update_fn(), this.progressUpdateInterval)
  }


  clearProgressTimer(update_fn) {
    update_fn()
    const hasExistingTimer = this.progressTimer
    if(hasExistingTimer) {
      console.warn("no progress timer to clear, yet a clear command was received.")

      return
    }
    console.log("[clearProgressTimer]", {
      timer: this.progressTimer,
    })
    clearInterval(this.progressTimer)
  }
}

@KosmonautX
Once the responsibilities are a little bit more separate (after doing the media bridge), I'll get back to this -- have stashed this implementation for now

@ks0m1c
Copy link
Contributor

ks0m1c commented Feb 26, 2024

I think one way to clean up the lifecycle is at syncProgressTimer currently you are clearing the Progress Timer if it exists if instead we manage its lifecycle differently to call it only when necessary guarding its execution for eg. we could

  syncProgressTimer(update_fn) {
    // Check if the timer is already active
    if (this.progressTimer) {
      console.log("Progress timer is already active.");
      return; // Exit HERE if the timer is already active
    }
    // Set up the timer
    this.progressTimer = setInterval(() => {
      update_fn();
      // we could even inject an exit condition here if needed
    }, this.progressUpdateInterval);
  }

Singleton Pattern is g as we are managing timer state in a global way but we do need to manage lifecycle with good precision when we intervene to avoid unintended side effects

Copy link
Contributor

@ks0m1c ks0m1c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very Clean High-Signal low noise PR LGTM

@ks0m1c ks0m1c merged commit 6380915 into chore/add-hanuman-chalisa-seeder Mar 3, 2024
ks0m1c added a commit that referenced this pull request Aug 19, 2024
# HitList

   
- [ ] Corpus Tasks: @KosmonautX 
  - [ ] Sivapuranam 🌹 [high priority] 
- [x] Hanuman Chalisa w multiple audio -- to focus on this first, so
that the workflow can be referred to and divided and conquered
- [x] Gita Chap 1 Audio & Events (defer to later, divide & conquer this)

- [ ] Media Sessions @rtshkmr 
    - [x] API should use image-gen img
    - [ ] Attributions to be added to the medium's meta
    - [x] fix video media stream playback issue
   - [ ]  Volume control
   - [x] Anchoring of video player for mobile view


- [ ] manual timestamping @KosmonautX 
  - [x] e.g. bookmarklet
🏴 [stretch goal] -- as a fallback, if this is not done, need to describe
- [ ] Sangha Integrations @KosmonautX 
  - [x] Sangh Session Routing - private **"circle of people"**
  - [x] Page Matrix
  - [x] Page Markup
  - [x] Integrate with Gita/Chalisa Verse Structure
  - [ ] Mark and drafting table event handling
  - [ ] Sangh Session Init upon first comment from draft (name)
  - [ ] prep for db and cleanup

- [x] Internal Pipelines @KosmonautX 
  - [x] chore: s3 integration for prod server
  - [x] prod migration chores


- [x] Admin console 
   - [x] mostly complete, just needs merging with the current webview
   - [x] @rtshkmr to review



---- 


- [x] Copy-System & Chores @rtshkmr 
- [x] ~Jekyll Site @KosmonautX~ -- changed to ox-hugo based copying for
ease of writing quickly
  - [x] [landing page](https://mmm.page/) init
  - [x] @rtshkmr init copy for sat


- [x] Image-gen: @rtshkmr 
    - [x] theme standardise 
    - [x] use a context-specific background


- [x] UI Polishing @rtshkmr 
    - [x] Follow Mode vs Non-Follow Mode
    - [x] Names to be prettified from snake_case
    - [x] General UI cleanups
      - [x] basic theming
      - [x]  mobile view (media player)

- [x] Hanuman Verse Struct & Events creation from existing .srt file (2
media inputs) @rtshkmr

- [x] Media Player Tasks: @rtshkmr 
  - [x] click verse to seek player #47 
- [x] cleanup: break the player live component into core components
instead.
- [x] Keep audio / video player as a child of bridge, bridge manages the
sync b/w the two players.
- [x] edge case: bufferring video ==> hide video, display toast to alert
user that their network is wonky (this is a depeer problem, will have to
get back to this. it's the mp3 issue) ==> this got handled by
@KosmonautX by making our audio files on S3 be stored at 320 bitrate
- [x] test mp4 containerising the mp3 for timestamp based seeking -- sep
independent child PR ==> abandoned because of the usage of higher
bitrate for the audio
- [x] Component positioning -- media bridge to be sticky and kept stuck
to bottom


#  Bugs List 
- [x] progress bar seek on pause #65 
- [x] sudden youtube iframe brokenness @rtshkmr 
- [x] Apple IOS bug for audio player init

# Abandon List 
- cleanup: decouple the timer from the audio player, likely a singleton
timer object that can be serialsed to store within the session storage,
so that playback can resume ([ref
comment](#41 (comment)))
[candidate for abandonment]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants