On Tue, Mar 08, 2016 at 11:27:45PM +0300, Pavel Nikiforov wrote:
> Nicolas George george at nsup.org wrote:
>
> >> libavformat/udp.c | 133
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >> 1 file changed, 129 insertions(+), 4 deletions(-)
>
> >Missing documentation update.
> fixed.
>
> >> - while(1) {
> >> + for(;;) {
>
> >Stray change.
> Portability. You don't know how a compiler other than clang or gcc
> will optimize or not optimize "while (1) { }".
>
>
> >> + if (!(h->flags & AVIO_FLAG_NONBLOCK)) {
>
> >This looks suspicious: blocking/nonblocking is a property of the protocol as
> >seen by the calling application, it should not apply in a thread used
> >internally. See below.
> The thread should work as original non-thread code.
>
> >> + ret = sendto (s->udp_fd, buf, size, 0,
> >> + (struct sockaddr *) &s->dest_addr,
>
> >Style: no spaces after function names and casts.
> It is not my code. This block from
> static int udp_write(URLContext *h, const uint8_t *buf, int size)
>
> >> + av_usleep(s->packet_gap);
>
> >Not very reliable: if the task gets unscheduled for some time, it will be
> >added to the gap and never catch, possibly causing playback hiccups. A
> >system with a kind of rate control would probably be better.
> Want reliable delay ? Spin in a loop polling dogettimeofday() in
> kernel w/o task switch. Otherwise the only thing we have is usleep() .
> No more options available to make a delay in user space: all of them
> cause task switch with unpredicted return time after delay (preemtable
> kernel, you knows).
>
> If all goes right and a system is not overloaded the gap between
> packets will be as set (and tcpdump with -ttt prove this). Also, we
> have a sendto() syscall (preemption again), the UDP socket buffer, a
> network device TX ring and many other things like "backpressure" on an
> ethernet switch port that will cause the delay be more than specified.
>
> >> + /*
> >> + Create thread in case of:
> >> + 1. Input and circular_buffer_size is set
> >> + 2. Output and packet_gap and circular_buffer_size is set
> >> + */
>
> >UDP sockets can be read+write; in this case, two threads are needed.
> The UDP socket are mostly read or mostly write. The thread will handle
> the most part of socket communication, no need to make a thread for
> handle some packets.
>
> Fixed version of the patch in attachment.> doc/protocols.texi | 3 + > libavformat/udp.c | 136 > +++++++++++++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 135 insertions(+), 4 deletions(-) > 04ecb9352d570991947f4481df8f3b3fcdba13a4 udp.patch > libavformat/udp.c | 136 > ++++++++++++++++++++++++++++++++++++++++++++++++++++-- > doc/protocols.texi | 3 +++ > 2 file changed, 135 insertions(+), 4 deletions(-) applied with alot of fixes i kept the code and most fixes in a seperate commit, felt more correct to me as i changed alot Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
