| Message ID | 20170523184402.25394-1-daniel.kucera@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Le quartidi 4 prairial, an CCXXV, Daniel Kucera a écrit : > Time to time some devices send UDP packets without payload. > ffmpeg previously exited on receiving such packet, this patch > fixes this behaviour. > > Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> > --- > libavformat/udp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) I do not think this is correct: UDP is a packetized protocol, packet with empty payload are still packets, and meaningful for some protocols. Regards,
2017-05-24 9:02 GMT+02:00 Nicolas George <george@nsup.org>: > > Le quartidi 4 prairial, an CCXXV, Daniel Kucera a écrit : > > Time to time some devices send UDP packets without payload. > > ffmpeg previously exited on receiving such packet, this patch > > fixes this behaviour. > > > > Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> > > --- > > libavformat/udp.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > I do not think this is correct: UDP is a packetized protocol, packet > with empty payload are still packets, and meaningful for some protocols. > > Regards, > > -- > Nicolas George Maybe, but in libavformat/async.c is zero treated as EOF: ret = ring_generic_write(ring, (void *)h, to_copy, wrapped_url_read); pthread_mutex_lock(&c->mutex); if (ret <= 0) { c->io_eof_reached = 1; if (c->inner_io_error < 0) c->io_error = c->inner_io_error; } So what do you suggest? S pozdravom / Best regards Daniel Kucera.
2017-05-24 9:02 GMT+02:00 Nicolas George <george@nsup.org>: > Le quartidi 4 prairial, an CCXXV, Daniel Kucera a écrit : >> Time to time some devices send UDP packets without payload. >> ffmpeg previously exited on receiving such packet, this patch >> fixes this behaviour. >> >> Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> >> --- >> libavformat/udp.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) > > I do not think this is correct: UDP is a packetized protocol, packet > with empty payload are still packets, and meaningful for some protocols. > > Regards, > > -- > Nicolas George If we return 0, then it is considered as EOF by calling functions (e.g. avio_read). So returning 0 means: "this is end of stream" and not: "we received zero length packet, but more data may be coming" and thus the stream processing ends. I need to fix this behavior, example errors are described here: https://blog.danman.eu/new-version-of-lenkeng-hdmi-over-ip-extender-lkv373a/#comment-478 S pozdravom / Best regards Daniel Kucera.
2017-05-23 20:44 GMT+02:00 Daniel Kucera <daniel.kucera@gmail.com>: > Time to time some devices send UDP packets without payload. > ffmpeg previously exited on receiving such packet, this patch > fixes this behaviour. > > Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> > --- > libavformat/udp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavformat/udp.c b/libavformat/udp.c > index 3835f989c4..28fa7b2354 100644 > --- a/libavformat/udp.c > +++ b/libavformat/udp.c > @@ -1039,7 +1039,11 @@ static int udp_read(URLContext *h, uint8_t *buf, int size) > av_fifo_generic_read(s->fifo, buf, avail, NULL); > av_fifo_drain(s->fifo, AV_RL32(tmp) - avail); > pthread_mutex_unlock(&s->mutex); > - return avail; > + if (avail > 0){ > + return avail; > + } else { > + continue; > + } > } else if(s->circular_buffer_error){ > int err = s->circular_buffer_error; > pthread_mutex_unlock(&s->mutex); > -- > 2.11.0 > Can anyone please review my patch? Thank you. S pozdravom / Best regards Daniel Kucera.
Le sextidi 6 prairial, an CCXXV, Daniel Kučera a écrit : > If we return 0, then it is considered as EOF by calling functions > (e.g. avio_read). So returning 0 means: "this is end of stream" and > not: "we received zero length packet, but more data may be coming" and > thus the stream processing ends. I need to fix this behavior, example > errors are described here: > https://blog.danman.eu/new-version-of-lenkeng-hdmi-over-ip-extender-lkv373a/#comment-478 Sorry for the delay, I was busy. FFmpeg inherits the inconsistency of the Unix API it imitates: 0 sometimes means EOF, but not always. But unlike Unix, FFmpeg has a code for explicit EOF, so it does not have to be inconsistent. But the correct way of making it consistent is to use AVERROR_EOF always, and never give a special role to 0. Your patch is going the opposite direction, and as such wrong. The right way is to find the places that use 0 as EOF and fix them. As is, this patch would make something that is currently possible with the API (implementing protocols that rely on empty packets) impossible, and that cannot be accepted. Regards,
On Sun, 28 May 2017 21:20:12 +0200 Nicolas George <george@nsup.org> wrote: > As is, this patch would make something that is currently possible with > the API (implementing protocols that rely on empty packets) impossible, > and that cannot be accepted. Which ones?
diff --git a/libavformat/udp.c b/libavformat/udp.c index 3835f989c4..28fa7b2354 100644 --- a/libavformat/udp.c +++ b/libavformat/udp.c @@ -1039,7 +1039,11 @@ static int udp_read(URLContext *h, uint8_t *buf, int size) av_fifo_generic_read(s->fifo, buf, avail, NULL); av_fifo_drain(s->fifo, AV_RL32(tmp) - avail); pthread_mutex_unlock(&s->mutex); - return avail; + if (avail > 0){ + return avail; + } else { + continue; + } } else if(s->circular_buffer_error){ int err = s->circular_buffer_error; pthread_mutex_unlock(&s->mutex);
Time to time some devices send UDP packets without payload. ffmpeg previously exited on receiving such packet, this patch fixes this behaviour. Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> --- libavformat/udp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)