On 25 August 2010 02:54, Martin Storsjö <[email protected]> wrote: > On Wed, 18 Aug 2010, Josh Allmann wrote: > > #3 looks ok, ok to commit, Luca/Ronald? >
No changes, but re-attaching anyway. If RDT doesn't make use of RTCP, then this patch makes no difference, since the RTCP fd is idle. This patch just makes RTCP reading a *bit* more robust, so that the RTSP layer will fetch a RTCP packet even when no RTP data packet is available. > #4 looks ok, I think, but could you redo it without reindenting the > unmodified lines, to make it clearer? > Done, now with a separate patch for cosmetics. > #5 raises a few questions and concerns, though. Earlier rtp_parse_packet > returned <0 for errors or RTCP (no output packet), 0 for ok packet, 1 for > ok packet and more packets following. Now it suddenly returns RTCP_BYE for > such RTCP packets (and returns 0 for RTCP_SR packets). This also makes > rtsp_fetch_packet return normally (as if a packet was returned, even if > none was) if a RTCP packet was received, instead of retrying to get a > proper data packet. > True. I thought about that, but I figured since the RTCP return types (200-204) aren't being used anywhere else, it was safe to propagate back up to the RTSP layer. > One way of fixing this would be to make rtcp_parse_packet return < 0 for > all cases, and return -RTCP_BYE for byes. The handling code would then > move up one step in rtsp_fetch_packet, to be within the "Either bad > packet, or a RTCP packet" block, after checking the first_rtcp_ntp_time > field. > Yes, this method is a bit more consistent with the current code. Fixed. > Also, I think it would feel a bit more robust if nb_byes was reset in > rtsp_read_play instead of in seek. > Indeed, that simplified things; the nb_byes = 0 initialization in ff_rtsp_connect is no longer needed. >> Autoexit does not trip under my tests, perhaps because with this >> patch, the EOF never makes it back to ffplay. Instead, >> av_read_frame_internal tries to return some (bogus?) final packets >> (utils.c:1119). > > That's due to the parser, which still may have outstanding data to return. > After returning this data, we go back to av_read_packet on the next call, > but then we still block on waiting for more packets in rtsp_fetch_packet. > You could add a > > if (rt->nb_byes == rt->nb_rtsp_streams) > return AVERROR_EOF; > > to the start of rtsp_fetch_packet, to take care of that case when the > function is called after returning eof once. > That worked, thanks. Josh > // Martin > _______________________________________________ > FFmpeg-soc mailing list > [email protected] > https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc >
From fa35efc8337eb83e252f1c5f80114127fd742366 Mon Sep 17 00:00:00 2001 From: Josh Allmann <[email protected]> Date: Fri, 13 Aug 2010 15:09:44 -0700 Subject: [PATCH 1/4] Read RTCP packets in RTSP demuxer. --- libavformat/rtsp.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index c228842..16997e3 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1653,7 +1653,7 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st, RTSPState *rt = s->priv_data; RTSPStream *rtsp_st; fd_set rfds; - int fd, fd_max, n, i, ret, tcp_fd, timeout_cnt = 0; + int fd, fd_rtcp, fd_max, n, i, ret, tcp_fd, timeout_cnt = 0; struct timeval tv; for (;;) { @@ -1670,12 +1670,12 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st, for (i = 0; i < rt->nb_rtsp_streams; i++) { rtsp_st = rt->rtsp_streams[i]; if (rtsp_st->rtp_handle) { - /* currently, we cannot probe RTCP handle because of - * blocking restrictions */ fd = url_get_file_handle(rtsp_st->rtp_handle); - if (fd > fd_max) - fd_max = fd; + fd_rtcp = rtp_get_rtcp_file_handle(rtsp_st->rtp_handle); + if (FFMAX(fd, fd_rtcp) > fd_max) + fd_max = FFMAX(fd, fd_rtcp); FD_SET(fd, &rfds); + FD_SET(fd_rtcp, &rfds); } } tv.tv_sec = 0; @@ -1687,7 +1687,8 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st, rtsp_st = rt->rtsp_streams[i]; if (rtsp_st->rtp_handle) { fd = url_get_file_handle(rtsp_st->rtp_handle); - if (FD_ISSET(fd, &rfds)) { + fd_rtcp = rtp_get_rtcp_file_handle(rtsp_st->rtp_handle); + if (FD_ISSET(fd_rtcp, &rfds) || FD_ISSET(fd, &rfds)) { ret = url_read(rtsp_st->rtp_handle, buf, buf_size); if (ret > 0) { *prtsp_st = rtsp_st; -- 1.7.0.4
From fd28a56205aea1af411a205f13899bffc20e5a8a Mon Sep 17 00:00:00 2001 From: Josh Allmann <[email protected]> Date: Fri, 27 Aug 2010 12:24:17 -0700 Subject: [PATCH 2/4] Read RTCP compound packets. --- libavformat/rtpdec.c | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-) diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index 83cc687..a05ca52 100644 --- a/libavformat/rtpdec.c +++ b/libavformat/rtpdec.c @@ -74,12 +74,28 @@ void av_register_rtp_dynamic_payload_handlers(void) static int rtcp_parse_packet(RTPDemuxContext *s, const unsigned char *buf, int len) { - if (buf[1] != RTCP_SR) - return -1; + int payload_len; + while (len) { + switch (buf[1]) { + case RTCP_SR: + if (len < 16) { + av_log(NULL, AV_LOG_ERROR, "Invalid length for RTCP SR packet\n"); + return AVERROR_INVALIDDATA; + } + payload_len = (AV_RB16(buf + 2) + 1) * 4; + s->last_rtcp_ntp_time = AV_RB64(buf + 8); if (s->first_rtcp_ntp_time == AV_NOPTS_VALUE) s->first_rtcp_ntp_time = s->last_rtcp_ntp_time; s->last_rtcp_timestamp = AV_RB32(buf + 16); + + buf += payload_len; + len -= payload_len; + break; + default: + return -1; + } + } return 0; } -- 1.7.0.4
From 8573a22be817ba577dfcb8c4b71679f4e064c67a Mon Sep 17 00:00:00 2001 From: Josh Allmann <[email protected]> Date: Fri, 27 Aug 2010 12:26:00 -0700 Subject: [PATCH 3/4] Cosmetics. --- libavformat/rtpdec.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index a05ca52..f0a7dae 100644 --- a/libavformat/rtpdec.c +++ b/libavformat/rtpdec.c @@ -84,10 +84,10 @@ static int rtcp_parse_packet(RTPDemuxContext *s, const unsigned char *buf, int l } payload_len = (AV_RB16(buf + 2) + 1) * 4; - s->last_rtcp_ntp_time = AV_RB64(buf + 8); - if (s->first_rtcp_ntp_time == AV_NOPTS_VALUE) - s->first_rtcp_ntp_time = s->last_rtcp_ntp_time; - s->last_rtcp_timestamp = AV_RB32(buf + 16); + s->last_rtcp_ntp_time = AV_RB64(buf + 8); + if (s->first_rtcp_ntp_time == AV_NOPTS_VALUE) + s->first_rtcp_ntp_time = s->last_rtcp_ntp_time; + s->last_rtcp_timestamp = AV_RB32(buf + 16); buf += payload_len; len -= payload_len; -- 1.7.0.4
From cf660ee6e5c4f59104c204e0e4097895b4fc8b10 Mon Sep 17 00:00:00 2001 From: Josh Allmann <[email protected]> Date: Sun, 15 Aug 2010 01:10:11 -0700 Subject: [PATCH 4/4] Add in RTCP BYE support. --- libavformat/rtpdec.c | 7 ++++--- libavformat/rtsp.c | 13 +++++++++++++ libavformat/rtsp.h | 5 +++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index f0a7dae..139cc17 100644 --- a/libavformat/rtpdec.c +++ b/libavformat/rtpdec.c @@ -92,11 +92,13 @@ static int rtcp_parse_packet(RTPDemuxContext *s, const unsigned char *buf, int l buf += payload_len; len -= payload_len; break; + case RTCP_BYE: + return -RTCP_BYE; default: return -1; } } - return 0; + return -1; } #define RTP_SEQ_MOD (1<<16) @@ -451,8 +453,7 @@ int rtp_parse_packet(RTPDemuxContext *s, AVPacket *pkt, if ((buf[0] & 0xc0) != (RTP_VERSION << 6)) return -1; if (buf[1] >= RTCP_SR && buf[1] <= RTCP_APP) { - rtcp_parse_packet(s, buf, len); - return -1; + return rtcp_parse_packet(s, buf, len); } payload_type = buf[1] & 0x7f; if (buf[1] & 0x80) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 16997e3..bacc038 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1226,6 +1226,7 @@ static int rtsp_read_play(AVFormatContext *s) char cmd[1024]; av_log(s, AV_LOG_DEBUG, "hello state=%d\n", rt->state); + rt->nb_byes = 0; if (!(rt->server_type == RTSP_SERVER_REAL && rt->need_subscription)) { if (rt->state == RTSP_STATE_PAUSED) { @@ -1777,6 +1778,9 @@ static int rtsp_fetch_packet(AVFormatContext *s, AVPacket *pkt) uint8_t buf[10 * RTP_MAX_PACKET_LENGTH]; RTSPStream *rtsp_st; + if (rt->nb_byes == rt->nb_rtsp_streams) + return AVERROR_EOF; + /* get next frames from the same RTP packet */ if (rt->cur_transport_priv) { if (rt->transport == RTSP_TRANSPORT_RDT) { @@ -1833,6 +1837,15 @@ static int rtsp_fetch_packet(AVFormatContext *s, AVPacket *pkt) rtpctx2->first_rtcp_ntp_time = rtpctx->first_rtcp_ntp_time; } } + if (ret == -RTCP_BYE) { + rt->nb_byes++; + + av_log(s, AV_LOG_DEBUG, "Received BYE for stream %d (%d/%d)\n", + rtsp_st->stream_index, rt->nb_byes, rt->nb_rtsp_streams); + + if (rt->nb_byes == rt->nb_rtsp_streams) + return AVERROR_EOF; + } } } if (ret < 0) diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h index 49dbfde..c6c3972 100644 --- a/libavformat/rtsp.h +++ b/libavformat/rtsp.h @@ -303,6 +303,11 @@ typedef struct RTSPState { /** RTSP transport mode, such as plain or tunneled. */ enum RTSPControlTransport control_transport; + + /* Number of RTCP BYE packets the RTSP session has received. + * An EOF is propagated back if nb_byes == nb_streams. + * This is reset after a seek. */ + int nb_byes; } RTSPState; /** -- 1.7.0.4
_______________________________________________ FFmpeg-soc mailing list [email protected] https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
