Skip to content

Minor fix change to copy semantics for const shared_ptr since it has …#104

Merged
alan-george-lk merged 1 commit intolivekit:mainfrom
siddimore:minor-fix
Apr 21, 2026
Merged

Minor fix change to copy semantics for const shared_ptr since it has …#104
alan-george-lk merged 1 commit intolivekit:mainfrom
siddimore:minor-fix

Conversation

@siddimore
Copy link
Copy Markdown
Contributor

@siddimore siddimore commented Apr 20, 2026

Summary

LocalAudioTrack::setPublication() and LocalVideoTrack::setPublication() both called std::move() on a const shared_ptr& parameter.
Because the parameter is const, std::move produces a const shared_ptr&&, which cannot bind to shared_ptr's
move-assignment operator (which requires non-const &&).
The compiler silently falls back to copy assignment — the std::move was a no-op.

This PR removes the std::move and documents why the virtual signature is left as const& rather than changed to
by-value (which would enable a true move but constitutes an API-breaking change).

What changed

local_audio_track.h — Replace std::move(publication) with direct copy assignment
local_video_track.h — Same fix
Added comments explaining the const& constraint from the base Track::setPublication virtual signature

Why not change the signature?

setPublication is declared virtual in Track. Changing the parameter from const shared_ptr& to shared_ptr (by value) would be an ABI/API break for any downstream code overriding this method.

Impact

No behavioral change — the code was already copying. This just removes a misleading std::move that suggests a move is happening when it isn't.

…2 overloads where copy assignment takes a const shared_ptr and increments ref count, with std:: move this defaults to a copy assignment because of const& shared_ptr
@siddimore siddimore marked this pull request as ready for review April 20, 2026 16:54
@xianshijing-lk
Copy link
Copy Markdown
Collaborator

Thanks for fixing it.

Copy link
Copy Markdown
Contributor

@alan-george-lk alan-george-lk left a comment

Choose a reason for hiding this comment

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

Thanks for this and great PR body/description of the issue!

@alan-george-lk alan-george-lk merged commit ada0b20 into livekit:main Apr 21, 2026
12 checks passed
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.

3 participants