[FFmpeg-soc] [PATCH] RTCP BYE

[Josh Allmann]

On 25 August 2010 02:54, Martin Storsjö <martin at martin.st> 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
> FFmpeg-soc at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Read-RTCP-packets-in-RTSP-demuxer.patch
Type: text/x-patch
Size: 2189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100827/a955d0c6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Read-RTCP-compound-packets.patch
Type: text/x-patch
Size: 1367 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100827/a955d0c6/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Cosmetics.patch
Type: text/x-patch
Size: 1115 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100827/a955d0c6/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-in-RTCP-BYE-support.patch
Type: text/x-patch
Size: 3165 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100827/a955d0c6/attachment-0003.bin>