refactor: Make ToxAV independent of toxcore internals. #2651

iphydf · 2024-02-07T22:47:56Z

This change is

codecov · 2024-02-07T22:56:40Z

Codecov Report

Attention: Patch coverage is 58.29596% with 186 lines in your changes are missing coverage. Please review.

Project coverage is 73.04%. Comparing base (102a1fa) to head (62159e2).

Files	Patch %	Lines
toxav/msi.c	55.40%	66 Missing ⚠️
toxav/rtp.c	57.14%	54 Missing ⚠️
toxav/toxav.c	71.90%	34 Missing ⚠️
toxav/bwcontroller.c	15.15%	28 Missing ⚠️
toxcore/tox.c	20.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2651      +/-   ##
==========================================
- Coverage   73.08%   73.04%   -0.04%     
==========================================
  Files         149      149              
  Lines       30531    30479      -52     
==========================================
- Hits        22313    22264      -49     
+ Misses       8218     8215       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

iphydf · 2024-02-11T10:47:22Z

This is ready for review.

toxcore/tox.c

toxav/toxav.c

Green-Sky

toxav_old.c uses the Messenger logger, instead the ToxAV logger.

edit: I see that they are still more tightly coupled to messenger and conferences. Maybe we just remove them for v0.3 ?

toxav/video.h

toxav/audio.h

toxav/rtp.c

Green-Sky · 2024-09-18T09:52:38Z

toxav/rtp.c

        free(session);
        return nullptr;
    }

    // First entry is free.
    session->work_buffer_list->next_free_entry = 0;

-    session->ssrc = payload_type == RTP_TYPE_VIDEO ? 0 : random_u32(m->rng);
+    session->ssrc = payload_type == RTP_TYPE_VIDEO ? 0 : rtp_random_u32(); // Zoff: what is this??


according to https://www.rfc-editor.org/rfc/rfc3550 , this is used to disambiguate multiple sessions. However we don't use this (yet?).

Green-Sky · 2024-09-18T09:59:00Z

toxav/toxav.c

        rc = TOXAV_ERR_NEW_MALLOC;
        goto RETURN;
    }

+    av->log = tox->m->log;


Green-Sky · 2024-09-20T07:56:15Z

toxav/bwcontroller.c


    if (length - 1 != sizeof(struct BWCMessage)) {
-        return -1;
+        LOGGER_ERROR(log, "Actual data size and length argument do not match");


sizeof(struct xxx) is an unreliable number that's implementation defined. We want 2 u32 instead.

Also, the log reads kinda wrong.

Green-Sky · 2024-09-20T08:00:09Z

toxav/msi.c

+#ifndef TOXAV_DEFINED
+#define TOXAV_DEFINED
+typedef struct ToxAV ToxAV;
+#endif /* TOXAV_DEFINED */


we are including toxav_hacks.h which provides functions that already need this, so it is safe to assume this already exists here.

Green-Sky · 2024-09-20T08:02:03Z

toxav/msi.c

-int msi_invite(MSISession *session, MSICall **call, uint32_t friend_number, uint8_t capabilities)
+
+/*
+ * return true if friend was offline and the call was canceled


is offline.

Green-Sky · 2024-09-20T08:24:04Z

toxav/bwcontroller.h

@@ -5,19 +5,24 @@
 #ifndef C_TOXCORE_TOXAV_BWCONTROLLER_H
 #define C_TOXCORE_TOXAV_BWCONTROLLER_H

-#include "../toxcore/Messenger.h"
+#include <stdint.h>


standard headers always after your own headers.

Green-Sky · 2024-09-20T08:32:38Z

toxcore/tox.h

@@ -3288,6 +3288,9 @@ bool tox_friend_send_lossless_packet(
    Tox_Err_Friend_Custom_Packet *error);

 /**
+ * tox_callback_friend_lossy_packet is the compatibility function to
+ * set callback for all packet IDs except those reserved for ToxAV


Green-Sky · 2024-09-20T08:34:26Z

toxcore/Messenger.h

-typedef void m_msi_packet_cb(Messenger *m, uint32_t friend_number, const uint8_t *data, uint16_t length,
-                             void *user_data);
-typedef int m_lossy_rtp_packet_cb(Messenger *m, uint32_t friend_number, const uint8_t *data, uint16_t length, void *object);
+typedef int m_lossy_rtp_packet_cb(Messenger *m, uint32_t friend_number, const uint8_t *data, uint16_t len, void *object);


len? -> length

iphydf added this to the v0.2.19 milestone Feb 7, 2024

iphydf marked this pull request as ready for review February 7, 2024 22:48

iphydf force-pushed the toxav-public-api branch 2 times, most recently from e59814d to 8a4092b Compare February 9, 2024 02:34

iphydf modified the milestones: v0.2.19, v0.2.20 Feb 11, 2024

iphydf force-pushed the toxav-public-api branch from 8a4092b to 55a8294 Compare February 28, 2024 16:17

iphydf marked this pull request as draft March 7, 2024 16:20

iphydf force-pushed the toxav-public-api branch from 55a8294 to e0896da Compare March 7, 2024 16:20

iphydf force-pushed the toxav-public-api branch from e0896da to 756ee54 Compare March 15, 2024 11:51

iphydf force-pushed the toxav-public-api branch 2 times, most recently from eb8a05a to 5b737a8 Compare March 28, 2024 15:22

zoff99 mentioned this pull request Apr 27, 2024

docs(toxav): fix docs of toxav.h #2754

Draft

Green-Sky reviewed Apr 27, 2024

View reviewed changes

toxcore/tox.c Outdated Show resolved Hide resolved

pull-request-attention bot assigned iphydf Apr 27, 2024

Green-Sky reviewed Apr 27, 2024

View reviewed changes

toxav/toxav.c Outdated Show resolved Hide resolved

Green-Sky force-pushed the toxav-public-api branch from 5b737a8 to 59d3feb Compare April 27, 2024 13:01

Green-Sky marked this pull request as ready for review April 27, 2024 13:03

iphydf force-pushed the toxav-public-api branch from 0f687cf to 62159e2 Compare May 1, 2024 13:31

Green-Sky force-pushed the toxav-public-api branch from 62159e2 to 664ee5f Compare September 17, 2024 18:42

refactor: Make ToxAV independent of toxcore internals.

560cd96

Green-Sky force-pushed the toxav-public-api branch from 664ee5f to 560cd96 Compare September 18, 2024 07:02

Green-Sky requested changes Sep 18, 2024

View reviewed changes

Green-Sky added 2 commits September 18, 2024 12:07

bring back the old doc fix

32f88ef

address some of my review 1

d4978a3

Green-Sky force-pushed the toxav-public-api branch from dac5f88 to d4978a3 Compare September 20, 2024 07:23

Green-Sky requested changes Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Make ToxAV independent of toxcore internals. #2651

refactor: Make ToxAV independent of toxcore internals. #2651

iphydf commented Feb 7, 2024 •

edited

Loading

codecov bot commented Feb 7, 2024 •

edited

Loading

iphydf commented Feb 11, 2024

Green-Sky left a comment •

edited

Loading

Green-Sky Sep 18, 2024

Green-Sky Sep 18, 2024

Green-Sky Sep 20, 2024

Green-Sky Sep 20, 2024

Green-Sky Sep 20, 2024

Green-Sky Sep 20, 2024

Green-Sky Sep 20, 2024

Green-Sky Sep 20, 2024

refactor: Make ToxAV independent of toxcore internals. #2651

Are you sure you want to change the base?

refactor: Make ToxAV independent of toxcore internals. #2651

Conversation

iphydf commented Feb 7, 2024 • edited Loading

codecov bot commented Feb 7, 2024 • edited Loading

Codecov Report

iphydf commented Feb 11, 2024

Green-Sky left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iphydf commented Feb 7, 2024 •

edited

Loading

codecov bot commented Feb 7, 2024 •

edited

Loading

Green-Sky left a comment •

edited

Loading