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

TEMP - ffmpeg output debugging #3406

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

TEMP - ffmpeg output debugging #3406

wants to merge 7 commits into from

Conversation

j0sh
Copy link
Collaborator

@j0sh j0sh commented Feb 20, 2025

No description provided.

j0sh and others added 4 commits February 20, 2025 05:33
* Reduce segmenter ffmpeg's analyzeduration to 2s. This avoids waiting for
  audio during RTMP pull at the risk of missing late tracks, especially video
  with a long GOP.

* Send current segment immediately when the publisher is ready to receive data,
  rather than waiting for the next segment. This reduces time-to-first-frame at
  the risk of higher *perceived* latency if playback happens to start at the
  current segment (which we are effectively buffering, instead of staying on the
  leading edge by waiting for the next segment)

* Rearrange retry sleeps in the segmenter: if ffmpeg needs to retry, it will
  immediately check for stream existence and exit if necessary. If the stream
  still exists, only then will it sleep for 5 seconds. This avoids a 5 second
  delay in signaling EOF.

* Add some comments around how retries are still kinda broken anyway

* Fix a couple of missing body closes in mediamtx HTTP request handling

* Log ffmpeg output on rtmp pull errors
@github-actions github-actions bot added go Pull requests that update Go code AI Issues and PR related to the AI-video branch. labels Feb 20, 2025
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 0% with 52 lines in your changes missing coverage. Please review.

Project coverage is 32.08888%. Comparing base (232df3a) to head (efcb9f3).

Files with missing lines Patch % Lines
server/ai_live_video.go 0.00000% 28 Missing ⚠️
media/rtmp2segment.go 0.00000% 8 Missing ⚠️
media/segment_reader.go 0.00000% 8 Missing ⚠️
server/ai_mediaserver.go 0.00000% 6 Missing ⚠️
media/mediamtx.go 0.00000% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3406         +/-   ##
===================================================
- Coverage   32.11405%   32.08888%   -0.02517%     
===================================================
  Files            147         147                 
  Lines          40789       40821         +32     
===================================================
  Hits           13099       13099                 
- Misses         26916       26948         +32     
  Partials         774         774                 
Files with missing lines Coverage Δ
media/mediamtx.go 0.00000% <0.00000%> (ø)
server/ai_mediaserver.go 7.27580% <0.00000%> (-0.01234%) ⬇️
media/rtmp2segment.go 3.38983% <0.00000%> (-0.08843%) ⬇️
media/segment_reader.go 0.00000% <0.00000%> (ø)
server/ai_live_video.go 0.00000% <0.00000%> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 232df3a...efcb9f3. Read the comment docs.

Files with missing lines Coverage Δ
media/mediamtx.go 0.00000% <0.00000%> (ø)
server/ai_mediaserver.go 7.27580% <0.00000%> (-0.01234%) ⬇️
media/rtmp2segment.go 3.38983% <0.00000%> (-0.08843%) ⬇️
media/segment_reader.go 0.00000% <0.00000%> (ø)
server/ai_live_video.go 0.00000% <0.00000%> (ø)

... and 2 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Issues and PR related to the AI-video branch. go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants