[FFmpeg-devel] Realmedia patch

[Luca Abeni]

Hi Ronald,

Ronald S. Bultje wrote:
> Hi,
> 
> On Wed, Sep 3, 2008 at 9:01 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> Anyway, seems you really want this, attached is a patch to generalize
>> the code around rtp_parse_open() (#1), so that I can make rtp_ctx
>> opaque as you requested (#2, #3).
> 
> This is not a good idea in this way, I'm now noticing. We're using the
> RTSP server reply to set it up, but deciding on the implementation
> based on the payload in the SDP.

This is the wrong thing, I'd say. The implementation (you mean RDT vs
RTP, right?) should be chosen based on the used "Transport:" attribute
in the RTSP messages, not based on the payload.


> This is exploitable by adding a
> RM-style payload in the SDP in an otherwise non-RM server or the other
> way around.

We can add a simple check to avoid using RM-style payloads in non-real
rtsp, or regular payloads in real rtsp. I think this is the simplest
and safest thing to do.


> I'm affraid I need to think about how to do this correctly
> also, but we need a single point where this is decided.

Well, the solution looks simple (see above).

> Right now, the
> APIs between RTP and RDT need to be completely compatible (i.e. it
> needs to act as a vfunc with implementation-compatible arguments,
> opaque (API-compatible) isn't good enough).

This is a problem. For making a type opaque, we must ensure that
everything is consistent (that is, the structure is not used by an
rdt function if it has been allocated by an rtp function, etc...).
Using opaque types only makes the problem more visible, but the
consistency problem exists also if you use the same structure in both
RTP and RDT.

> Alternatively, I can remove my current subscription patch, which adds
> RDT to RTPDynamicPayloadHandlers, and use the same style to create a
> RDTDynamicPayloadHandlers list (which is the same API, just a
> different list).

I suspect this is not needed: we just have to be consistent in
calling RTP and RDT functions, and to remove RTP assumptions from
rtsp.c.
I think the simple check suggested above should be enough, but I did
not fully check all the details.


				Luca