Closed maceyldn closed 5 years ago
Hi Andy,
thanks for your MR! I will (hopefully) be able to review it at the weekend. I might ask you to do slightly adjustments to your contribution before merging it ;-)
Regards, Stefan
LOL No Problem at all.
I fully expect it. My C is terrible!
Andy
Some first findings/questions; I haven't checked the main part yet.
next
branch, as it contains the latest changes (the master
branch is the last release version and usually a bit older)next
) for a particular change, e.g. feature_ip_input
gnu++11
? If c++11
works as well, I would prefer it (the previous c++0x
seems to be deprecated); in general, this change should be in a separate commit, as it is independent from the IP inputeti_player.cpp
was changed, but only empty lines were addeddoc
subfolder) and (with usage examples in) README.md
-f
when using a file. A possible alternative would be to just add a parameter -u
that implies that the specified string is not a file but an URLI hope all this is not too frustrating ;-) I think it makes sense for you to wait with any changes until the "main review" before applying changes.
Sorry for the delay; I just had a further look at your changes. In general your changes add the ability to play from a TCP source. In addition to the above mentioned issues, I found the following:
There is a big "if" in eti_source.cpp
to handle the URL case instead of the file case. This is not really good style. As the URL/file ways have many commonalities, it makes sense to use the C++ inheritance mechanism to have a base class. From this base class two sub-classes could inherit and handle the URL and the file cases differently i.e. at the code location where different code would be used, a function is called that is implemented differently for each sub-class (e.g. init, reading etc.).
Sometimes a plain exit
call is used, instead of return
. This prevents proper clean-up of the current state and should not be used.
Sometimes empty lines are added/removed without real need. The change should be kept as minimal as possible, so that the changes remain clear.
In general, using a separate application to access a TCP source (e.g. netcat
) more aligns with the KISS principle. Furthermore, since over a year there has been no update here. For this reasons I close this MR and hope, this is OK for you.
C is sadly not my language... (you will see when you look at the code ;) )
but under the windows build i created, i needed the ability to recieve data through a TCP port without external apps.
This enables that, slight difference to the command line options, but that is explained in --help
It also contains some modifications to build under Cygwin.
Regards Andy