[FFmpeg-soc] [PATCH] RTCP BYE

[Martin Storsjö]

On Wed, 25 Aug 2010, Ronald S. Bultje wrote:

> On Wed, Aug 25, 2010 at 5:54 AM, Martin Storsjö <martin at martin.st> wrote:
> > #2 looks ok, ok to commit, Luca/Ronald?
> 
> OK.

Applied

> > #3 looks ok, ok to commit, Luca/Ronald?
> 
> Doesn't that include the RTCP filehandle for RDT-over-UDP as well? It shouldn't.

It does, but it doesn't change much from how it works currently.

Currently, we have an rtp:// urlcontext open even for RDT, and if a packet 
is sent to the RTCP port of that urlcontext, it will be returned. On the 
higher level in rtsp.c, we select among the fd's of the urlcontexts, and 
using that choose which urlcontext to url_read() from.

So currently, we only try to read from an urlcontext if there's a packet 
waiting on the RTP port, but once that happens, the rtp url_read() may 
just as well return a packet from the RTCP port. This works just the same 
for RDT too. What Josh's patch #3 changes is only that we check the RTCP 
fd for packets, too, on the higher level, so that we may read from the 
RTCP port even if there's no packet pending on the RTP port.

So, if the previous behaviour is ok for RDT, the new one should be just as 
acceptable, as far as I understand it.

> > #4 looks ok, I think, but could you redo it without reindenting the
> > unmodified lines, to make it clearer?
> 
> I'm actually wondering if we shouldn't make RTCP packet parsing a
> separate public function called from rtsp.c. That way, rtsp.c knows
> better whether we're dealing with RTCP or RTP data.
> rtsp_fetch_packet() makes some pretty bad assumptions based on that
> now, all of that might be clean-uppable if we fix this by telling the
> caller which FD the packet came from.

Hmm, yes, this may have some benefits, but I'm not sure off-hand how much 
work it would need to untangle it.

When receiving the packet over UDP from an rtp:// urlcontext, the current 
API doesn't expose which of the ports the packet was received on, so we'd 
still need to resort to the ugly introspection of the second byte of the 
packet to check whether it's RTP or RTCP. For TCP interleaving, it should 
be quite possible to pass that info on, with the current abstractions, 
though.

// Martin