Additional RemoteTrackPublication options - #219
Conversation
|
We’ve verified this functionality in our integration, including |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| width_ = width; | ||
| height_ = height; |
There was a problem hiding this comment.
🟡 Requesting a smaller video size makes the app report the wrong published resolution
The publication's reported video size is overwritten with the size the app asked to receive (width_ = width at src/remote_track_publication.cpp:160-161) instead of being kept separate, so apps reading the publication's resolution get the requested value rather than the resolution actually published by the sender.
Impact: UI or logic that relies on a remote publication's advertised video resolution silently shows incorrect numbers after a dimension request, and the original resolution is lost forever.
Metadata fields shared between publication info and the new dimension-preference state
width_/height_ are the protected publication-info members populated from the server's TrackInfo in the constructor (include/livekit/track_publication.h:83-84, exposed via width()/height() at include/livekit/track_publication.h:54-55). setVideoDimensions() reuses them both as the dedup key (src/remote_track_publication.cpp:141) and as the storage for the requested values (src/remote_track_publication.cpp:160-161). Nothing else ever writes these fields (no other setter exists), so once a request is made the actual published dimensions are unrecoverable. The new integration test even asserts the corrupted values (src/tests/integration/test_remote_track_publication.cpp:80-81). The JS SDK keeps requested dimensions in a separate videoDimensions field rather than mutating the publication's track info.
Prompt for agents
In RemoteTrackPublication::setVideoDimensions (src/remote_track_publication.cpp), the requested receive dimensions are stored into the base class's width_/height_ members, which hold the publication info reported by the server and are exposed publicly through TrackPublication::width()/height(). This destroys the real published resolution and makes those accessors report the app's request instead. Introduce private members on RemoteTrackPublication (e.g. requested_width_/requested_height_) to track the last requested dimensions, use them for the dedup check currently at line 141, and optionally expose them via a new accessor. Update the unit/integration tests that currently assert width()/height() equal the requested values.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// or the publication is not a subscribed video track. | ||
| /// @throws std::runtime_error if the publication has an invalid FFI handle or | ||
| /// the FFI request fails. | ||
| bool setVideoDimensions(std::uint32_t width, std::uint32_t height); |
There was a problem hiding this comment.
const std::uint32_t width ?
| /// the publication is not a subscribed, simulcasted video track. | ||
| /// @throws std::runtime_error if the publication has an invalid FFI handle or | ||
| /// the FFI request fails. | ||
| bool setVideoQuality(VideoQuality quality); |
There was a problem hiding this comment.
const VideoQuality quality ?
| /// the publication is not subscribed. | ||
| /// @throws std::runtime_error if the publication has an invalid FFI handle or | ||
| /// the FFI request fails. | ||
| bool setEnabled(bool enabled); |
There was a problem hiding this comment.
const bool enabled?
|
|
||
| const proto::FfiResponse resp = FfiClient::instance().sendRequest(req); | ||
| if (!resp.has_enable_remote_track_publication()) { | ||
| throw std::runtime_error("RemoteTrackPublication::setEnabled: FFI response missing enabled update result"); |
There was a problem hiding this comment.
i get throwing for ffiHandleId() == 0 but do want to throw if we miss a response? Probably, otherwise we have broken our contract with the ffi?
| return false; | ||
| } | ||
| if (video_preference_ == VideoPreference::QUALITY && video_quality_ == quality) { | ||
| return false; |
There was a problem hiding this comment.
does this means the quality that is desired is already set? If so, should we instead return true here? logic being they are getting the intended result of the function call.
| return false; | ||
| } | ||
| if (video_preference_ == VideoPreference::DIMENSIONS && width_ == width && height_ == height) { | ||
| return false; |
There was a problem hiding this comment.
same return true comment here
3 participants
Features generated from issue #212
RemoteTrackPublicationoptions that better match the JS SDK where possible (FFI-supported only): https://github.com/livekit/client-sdk-js/blob/main/src/room/track/RemoteTrackPublication.ts