openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.94k stars 2.55k forks source link

why can't we set "UdpSocketWin.cpp" the MAX_BUFFER_SIZE a bit bigger? #3106

Closed QiangJi closed 9 years ago

QiangJi commented 10 years ago

@ofTheo at "404 const int MAX_BUFFER_SIZE = 4098;"

bilderbuchi commented 10 years ago

To clarify, you are talking about this line for windows only. Interestingly for posix, it's much bigger. This is oscpack code (i.e. 3rd party library), I would think there is a reason why it's set that way, and I'd be hesitant to change it.

QiangJi commented 10 years ago

@bilderbuchi If so, we OF the osc example blobArg bytes sent more than 4098 can't send success.

xiajiaonly commented 10 years ago

@bilderbuchi
I think changing this value is not the ultimate way to solution the problem. The problem is this:When the size of sent data over this value, OSC will not receive this data. Since the transmitted data size is not limited, so that the data size can be very large, although generally not large. Although the large data is poorly designed , this is not the reason for the existence of this bug. I think that the fundamental solution of this problem is to use automatic pack mechanisms. We can achieve this mechanism at many levels. But I think it is better to achieve this mechanism in OscPack. Just modify this value , then how much is appropriate? This is a problem , too large value will cause unnecessary memory consumption.

bilderbuchi commented 10 years ago

But I think it is better to achieve this mechanism in OscPack.

Then this should be reported to Oscpack, imo.

xiajiaonly commented 10 years ago

@bilderbuchi I think so,but my English is poor...

bilderbuchi commented 10 years ago

Not that poor I think. you can also include a link to this bug report, too, for additional context. However, let's just ping the oscpack developer, @RossBencina, and see what he thinks. :-)

RossBencina commented 10 years ago

Hi Guys,

Four things:

  1. This bug report provides no explanation for why the request is being made. That makes it difficult for me to give a meaningful response. Apologies in advance if I've missed the point.
  2. The large value of MAX_BUFFER_SIZE on the oFx posix version seems like some kind of oFx specific hack. MAX_BUFFER_SIZE is 4096 for both Windows and Posix in the oscpack trunk.
  3. This code in question is for UDP. UDP packets generally need to be smaller than the MTU, which is usually less than 1500 bytes. If the goal is to support Jumbo frames then you could increase it to 9000, see https://en.wikipedia.org/wiki/Jumbo_frame Not all networks support Jumbo Frames.
  4. From my point of view, the bottom line is this: if you are using OSC over UDP, and you want your code to work on arbitrary networks then you must keep your packets smaller than the MTU. OSCpack does not enforce this, but 4096 is a very generous over-allocation for the temp buffer given that your network will likely not pass packets >1500 bytes.

Let me know if I've missed something. Thanks.

ofTheo commented 10 years ago

This was something I changed recently in ofxOsc. I wanted to be able to send larger binary blob data with the ofxOsc example than 4096. ( so I could send small images over OSC )

I dealt with the MTU I believe with these lines here which increase the socket send size: https://github.com/openframeworks/openFrameworks/blob/5f79889ecd85768018d2bd2a82131835ad923ebd/addons/ofxOsc/libs/oscpack/src/ip/posix/UdpSocket.cpp#L131

We could try doing the same on Windows or maybe better add a feature to ofxOsc to allow setting buffer and MTU size, but put the default back to 4096 so its at least a bit more reasonable as a default?

Having said that is there really any harm in have a larger buffer. I can't imagine the overhead is that much greater and it only becomes an issue of compatibility if you try and use the full buffer ( which most of the time you won't be )

xiajiaonly commented 10 years ago

@ofTheo Add a function to set this value is better than directly modify this value. But....... Usually as a program developer, we know that the maximum value of this number . But sometimes we may not be determined how much the maximum of this value is.

  1. At this point we need to check the data sent in the code and give some appropriate tips , otherwise it will cause confusion .
  2. Sometimes sending and receiving ends are developed by different people , they must know in advance the maximum limit .In most cases there will be no problems, but problems may occur in extreme cases .
  3. Development of a third-party tool based on osc . we are not sure how much the maximum is,We had to use a larger value , in most cases this value is irrelevant . But in extreme cases this value is not enough, we had to use an interface to allow the user to set this value , and let them understand the meaning of this value .

In summary I think the increase in the interface is not a perfect solution.

RossBencina commented 10 years ago

@ofTheo Changing the buffer sizes on the local socket doesn't magically ensure that larger packet sizes will work. The MTU is determined by all devices between sender and receiver (routers, the other host, etc). See for example:

http://stackoverflow.com/questions/1098897/what-is-the-largest-safe-udp-packet-size-on-the-internet https://en.wikipedia.org/wiki/Maximum_transmission_unit#IP_.28Internet_protocol.29

Max message size on Windows is 65507: http://support.microsoft.com/kb/822061

That doesn't mean that the message will make it across the network.

I maintain that if your packets are larger than the standard LAN MTU then you're doing it wrong. You need to redesign your application protocol, not use unreliable hacks. Giving oFx users the ability to do unreliable hacks as if it was normal practice is a Bad Thing (TM).

I agree with @xiajiaonly that if the buffer size is be made bigger, it should be done with a setter/getter, not by default. This has the advantage that the method can be documented: "be aware that using larger UDP packets may not work on your network."

RossBencina commented 10 years ago

@xiajiaonly out of interest, what applications have you seen that send UDP OSC packets larger than 4096 bytes?

yty commented 10 years ago

@RossBencina

I think it can be done Ventuz.OSC....

http://opensoundcontrol.org/implementation/osc-net-v1-2

A free OSC library for Microsoft.Net with a full implementation of the OSC protocol. Extends the protocol to be able to transfer images and Unicode encoded strings as a blob element.

xiajiaonly commented 10 years ago

@RossBencina ofTheo 's answer already mentioned this application ,"binary blob data or small image". I have no application in this regard. Just to post my views against this problem.

bilderbuchi commented 10 years ago

@RossBencina thank you for pitching in. Sorry, I previously didn't realize that on the posix side maxbuffer was patched by us, in 3f60774f4534a12d9d449120493b4fbb19cc9319.

I agree that the best course would probably be to, if we need this, have it settable with some function, and include appropriate warnings/usage info in an accompanying doxygen comment. that way, if an advanced user knows the MTU of his/her Network to be big enough, it can be set, without potentially impacting the performance for "normal" use. I don't think we should expand on the OSC specification, otherwise this just begs for trouble when OF interacts with other OSC software.

also, +1 on not doing hacky things with network communication.

ofTheo commented 10 years ago

I agree with everything said here.

I am going to revert the current hardcoded values for OS X to the default and add an option to increase the size with a function call. Of course the call won't be guaranteed, but it could be useful in some circumstances.

RossBencina commented 10 years ago

For what it's worth, I did ask about this on the osc-devel mailing list:

http://comments.gmane.org/gmane.comp.audio.osc.devel/1439

There wasn't really consensus, but Matt Wright's view was that up to 2^16 should be supported. That's the limit of the UDP packet size field.

https://en.wikipedia.org/wiki/User_Datagram_Protocol

So yeah, a function call to change the buffer size makes sense. Leave the current value as the default.

Please let me know when it's done and I'll merge it upstream.

bilderbuchi commented 9 years ago

Closed by #3210

micuat commented 9 years ago

Hi @ofTheo,

add an option to increase the size with a function call.

Is it implemented? I'm using v20150910 nightly + VS2015 but the oscSend/Receive examples do not work unless MAX_BUFFER_SIZE is changed.