irungentoo / toxcore

The future of online communications.
https://tox.chat/
GNU General Public License v3.0
8.74k stars 1.27k forks source link

New API: Shouldn't it have a way to tell the avatar image format? #1264

Open ittner opened 9 years ago

ittner commented 9 years ago

The new API proposal for avatar transfer (as in https://github.com/irungentoo/toxcore/blob/af881e820a3f4b3d8f629c1cfd192fed049254f7/toxcore/tox.h#L1381 ) lacks a way to tell the image data format to the other peer (currently only NONE or PNG, as happens with the enums from the current API, but we should provide for new formats in the future without breaking the protocol).

I suggest adding the format as the first byte of the file transfer, so we have the following format: [uint8_t 1: image format][[uint8_t 32: SHA256][uint8_t ...: image data]]

And clients may announce the removal or non-existency of an avatar with a single-byte file transfer.

irungentoo commented 9 years ago

How about just adding more formats to the enum like:

TOX_FILE_KIND_AVATAR_PNG, TOX_FILE_KIND_AVATAR_WHATEVER

ittner commented 9 years ago

How about just adding more formats to the enum like:

It is possible (and avoids too much complexity in the avatar file transfer sub-protocol), but I still favor the type in the data stream approach, as it allows an application to distinguish between "peer tried to send me something I don't understand" and "peer tried to send me an avatar in a format I don't understand" and, more importantly, avoids using several file kinds for avatars.

The second reason is important to prevent a combinatorial explosion of file kinds if we reuse this approach for other automated file transfers that must also handle data formats, as recorded audio messages, extended profiles, friend lists synchronization (I was investigating this as a first step to multi-device connection, but it went nowhere as I couldn't think a way of doing multi-connection without breaking the DHT).