irtlab / rtptools

RTP Tools
http://www.cs.columbia.edu/irt/software/rtptools/
Other
166 stars 64 forks source link

replace hsearch() with a simple list #89

Closed janstary closed 6 years ago

janstary commented 6 years ago

Currently, rtpplay uses a hash table to remember the SSRCs we have seen and their timestamps, to be used when playing according to the timestamps. That's overkill: we don't need a full-fledged hash table implementation. This diff replaces that with a simple list.

Then we don't need to check for a hsearch(3) implementation, don't need to use the bundled buggy 1993 implementation (glibc 2.0.4), and can get rid of one more dependency.

Tested with the bark.rtp in the distribution; needs more testing with more than one SSRC of course.

Please test everywhere.

janstary commented 6 years ago

If merged, this closes https://github.com/columbia-irt/rtptools/issues/73

mcd500 commented 6 years ago

This pull request have two objectives, one is removing white space from the end of line, and replacing hsearch() with a simple list. Could you separate them to different pull requests, to make it easier to review by reducing the lines of changes for each pull request. And also the rtptrans has own list function buit-in.

typedef struct stream_id{
  int seq;
  int addr;
  int next_ts;
  struct stream_id* next;
} STREAM;

It would be nice to consolidate the both with the nice simple list in this pull request.

PS: I will review the others next week. Just have things to do.

janstary commented 6 years ago

Whitespace separated into https://github.com/columbia-irt/rtptools/pull/94. I will look at what rtptrans does.

janstary commented 6 years ago

rtptrans keeps a list of the streams arriving on a multicast link and their sequence numbers when playing to a unicast network.

That's not what rtpplay does, although technically they are both lists of course. I would let each one use its own: there is imho nothing to gain by abstracting a list with an opaque data pointer or similar.

Let rtptrans have its list of streams and seqeunce numbers, and let rtpplay have its list of ssrcs and timestamps.

The point here is to get rid of hsearch. But by now the branches are in conflict.

janstary commented 6 years ago

A clean version prepared in https://github.com/columbia-irt/rtptools/pull/109