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

feat: support HLS media playback #25174

Merged
merged 2 commits into from
Sep 24, 2024
Merged

feat: support HLS media playback #25174

merged 2 commits into from
Sep 24, 2024

Conversation

daibhin
Copy link
Contributor

@daibhin daibhin commented Sep 24, 2024

Problem

HLS media playback is not supported in Replay right now because the src is not added to the DOM and thus not captured by rrweb

Changes

Implementation as described in #25087 (comment)

Key thing to note is that the media source must be added as a hls-src attribute on the video element (needs to be added to the docs once this is testred)

How did you test this code?

Tested video plays back locally

Before After
before after

Copy link
Contributor

github-actions bot commented Sep 24, 2024

Size Change: +87 B (+0.01%)

Total Size: 1.1 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.1 MB +87 B (+0.01%)

compressed-size-action

@daibhin daibhin requested a review from a team September 24, 2024 14:14
Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

other than the question about the url, this is a great fix

@daibhin daibhin merged commit bfcf27e into master Sep 24, 2024
93 checks passed
@daibhin daibhin deleted the dn-feat/hls-playback branch September 24, 2024 15:34
@MannyAdumbire
Copy link

@daibhin thanks for the quick fix!

Unfortunately I'm not sure how to test this is working, using the "hobby" docker version of PostHog.
How would I confirm that my local copy of PostHog has loaded this PRs changes?

I added the hls-src attribute on <video> element with .m3u8 HLS video for its value.

Then did the following:

  1. pull the latest changes from master branch which contains this merged PR.
  2. ran docker-compose up --build -d
  3. "record" a couple sessions for replay

Still seeing the original issue after the steps above.

I searched(greped) inside the web, caddy, plugins, worker docker containers, but can't find any reference to "hls-src".
I'm assuming it will take time to be deployed to PostHog cloud, so really appreciate any pointers as to where/how this PR changes would show up on the hobby version.

@daibhin
Copy link
Contributor Author

daibhin commented Oct 1, 2024

Hey @MannyAdumbire, do you have a link to one of the affected recordings? Just trying to recreate this locally on my machine but might be quicker to use one of your examples if you have one to hand

@daibhin
Copy link
Contributor Author

daibhin commented Oct 1, 2024

You're right @MannyAdumbire, I hadn't anticipated that a HLS video would add a src attribute of it's own which is causing an exception to be thrown during playback. The fix requires an update to the SDK so you will need to upgrade once v1.166.1 is live

@MannyAdumbire
Copy link

MannyAdumbire commented Nov 4, 2024

@daibhin I finally figured out how to update the hobby version container to get built with the latest posthog-js.
HLS playback working as expected now. 😭:)
thanks again for your help!

@daibhin
Copy link
Contributor Author

daibhin commented Nov 4, 2024

Amazing! Did it require any changes on your end beyond upgrading the version? I would love to update our docs if there's anything that might help others to make this work

@MannyAdumbire
Copy link

MannyAdumbire commented Nov 12, 2024

Amazing! Did it require any changes on your end beyond upgrading the version? I would love to update our docs if there's anything that might help others to make this work

Sorry that I don't quite remember the key steps 😅
It went something like:

> cd /path/to/posthog/repo
> git pull
> docker build -t  posthog:latest . 
> docker-compose up -d --force-recreate --build

I wish I know what the minimal list of docker services needing to be rebuilt is, but each one I tried complained with some docker message about dependent container being affected.
I will have to take note next times.

This rough upgrade worked fine for a while but then I run into an issue that seemed to only resolve with the full upgrade mentioned in #25262 (comment)
https://posthog.com/docs/self-host#upgrading.
Running the full script wasn't too bad either time-wise to get back up running, other than it wiping some custom configs for my reverse-proxy setup.
Gonna try to avoid losing those configs time by moving them into a docker-compose.override.yml.

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.

4 participants