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

function names misleading in ToxAV #625

Closed
zoff99 opened this issue Nov 25, 2017 · 1 comment
Closed

function names misleading in ToxAV #625

zoff99 opened this issue Nov 25, 2017 · 1 comment
Labels
documentation Changes to the documentation P2 Medium priority toxav Audio/video
Milestone

Comments

@zoff99
Copy link

zoff99 commented Nov 25, 2017

these 2 seem like the exact opposite functions:

rtp_send_data(RTPSession *session, const uint8_t *data, uint16_t length, Logger *log)

handle_rtp_packet(Messenger *m, uint32_t friendnumber, const uint8_t *data, uint32_t length, void *object)

they have the same input params const uint8_t *data, uint16_t length

rtp_send_data seems to send a full RTP Message, and on the receiver handle_rtp_packet packet would then get the exact RTP Message to handle it.
but thats not the case. there is actually no function that gets the full RTP Message on the receiver side.

handle_rtp_packet calls --> session->mcb
session->mcb() == vc_queue_message() // more confusing names
vc_queue_message() calls --> rb_write() // here you save parts of the RTP Message
vc_iterate() calls --> rb_read() // here you use parts of the RTP Message

and also the Packet ID byte gets silently stripped in handle_rtp_packet()
so if somebody in the future ever wants that information downstream
in vc_iterate() it is already lost.

these 2 things could be fixed to make it easier for people to understand and write new features

@zoff99
Copy link
Author

zoff99 commented Nov 25, 2017

now i think this comment is wrong:

c-toxcore/toxav/video.c

Lines 186 to 190 in 0fce3fc

int vc_queue_message(void *vcp, struct RTPMessage *msg)
{
/* This function does the reconstruction of video packets.
* See more info about video splitting in docs
*/

but i am not sure. would be good to clarify and comment in the code if actually the full RTP Message is given to vc_queue_message()

@SkyzohKey SkyzohKey added documentation Changes to the documentation toxav Audio/video P2 Medium priority labels Nov 28, 2017
@SkyzohKey SkyzohKey added this to the v0.2.0 milestone Nov 28, 2017
@iphydf iphydf modified the milestones: v0.2.0-RC1, v0.2.0 Jan 14, 2018
@iphydf iphydf modified the milestones: v0.2.0, v0.2.x Feb 19, 2018
@iphydf iphydf modified the milestones: v0.2.1, v0.2.x Mar 9, 2018
@zoff99 zoff99 closed this as completed Nov 4, 2018
@robinlinden robinlinden modified the milestones: v0.2.x, v0.2.9 Jan 3, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Changes to the documentation P2 Medium priority toxav Audio/video
Projects
None yet
Development

No branches or pull requests

4 participants