Merged
Conversation
Contributor
Author
|
This branch can be used for testing tiny-pilot/ustreamer-debian#23 (i.e., by installing the bundle off this branch on device). |
jotaen4tinypilot
added a commit
to tiny-pilot/ustreamer-debian
that referenced
this pull request
May 5, 2025
Related tiny-pilot/tinypilot-pro#1483. This PR upgrades the uStreamer library from 5.43 to 6.36 (the latest as of this PR). [uStreamer’s `6.24` release](pikvm/ustreamer@v6.23...v6.24) (specifically in [`config.c`](pikvm/ustreamer@v6.23...v6.24#diff-e5fe0ac4847bcb988b61f9e93b0d6ac773f22b564b5dd96ac53e5005632a895a) ) introduced breaking changes in the structure of the uStreamer<>Janus plugin configuration file: - `memsink.object` is now `video.sink`. - `audio` is now `acap` (presumably “audio capture”) - There is a new section available: `aplay` (“audio playback”? Not sure…). This seems to be related to microphone support, at least it doesn’t seem to be needed for just *playing* audio. On a Voyager 2a device, a full working `/etc/janus/janus.plugin.ustreamer.jcfg` would look like this: ``` video: { sink = "tinypilot::ustreamer::h264" } acap: { device = "hw:1" tc358743 = "/dev/video0" } ``` We’ll have to take over these changes [when writing the config file during install](https://github.com/tiny-pilot/tinypilot/blob/a9e7096c25a222741c25060776285e5f6ed43b51/debian-pkg/debian/tinypilot.postinst#L123-L139), see tiny-pilot/tinypilot#1881. <a data-ca-tag href="https://codeapprove.com/pr/tiny-pilot/ustreamer-debian/23"><img src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review on CodeApprove" /></a> --------- Co-authored-by: Jan Heuermann <jan@jotaen.net>
Contributor
Author
Automated comment from CodeApprove ➜⏳ @db39 please review this Pull Request |
db39
previously requested changes
May 19, 2025
Contributor
db39
left a comment
db39
left a comment
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved
LGTM!
In: Discussion
Is it worth updating the details in the user-configurable setting docs to document that ustreamer_desired_fps is specifically for MJPEG streaming?
👀 @jotaen4tinypilot it's your turn please take a look
db39
approved these changes
May 19, 2025
Contributor
db39
left a comment
db39
left a comment
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
Approved: I have approved this change on CodeApprove and all of my comments have been resolved.
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
3 participants
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.
Resolves https://github.com/tiny-pilot/tinypilot-pro/issues/1483.
Blocked on tiny-pilot/ustreamer-debian#23.(Resolved.)This PR upgrades our uStreamer dependency to the latest available version, from 5.43 to 6.36. There are two relevant things that have changed for us between versions, which we have to account for in our client code.
Dropped FPS for H264 (uStreamer 5.59)
uStreamer’s 5.59 release took away support for setting the frame rate in H264 mode, as discovered in https://github.com/tiny-pilot/tinypilot-pro/issues/1483#issuecomment-2877655846. So from 5.59 onwards, the frame rate setting is ignored in H264 mode, but only respected in MJPEG mode.
We decided to take away the frame rate slider in H264 therefore. We might introduce a new slider for setting the GOP value, which provides a sort-of equivalent functionality, but that is deferred to later.
I slightly adjusted our internal code semantics around the FPS setting, to reflect that it’s now MJPEG-specific. Unfortunately, this produced some code churn, but I still think it’s best to do this now, to make the code consistent.
Changed Config File Structure (uStreamer 6.24)
uStreamer’s 6.24 release introduced breaking changes in the structure of the Janus<>uStreamer plugin config file, as discovered in https://github.com/tiny-pilot/tinypilot-pro/issues/1483#issuecomment-2847012867. This PR migrates our config to the new format.