[FFmpeg-devel] [PATCH] RDT/Realmedia patches #2

[Ronald S. Bultje]

Hi Luca,

On Wed, Oct 1, 2008 at 2:00 AM, Luca Barbato <lu_zero at gentoo.org> wrote:
> Ronald S. Bultje wrote:
>> On Tue, Sep 30, 2008 at 9:42 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>>> On Tue, Sep 30, 2008 at 9:36 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>>>> As always, my full applied patch-tree against trunk SVN is at [1], I
>>>> hope you can see it shrinking over time.
>>> [..]
>>>> [1] http://www.gnome.org/~rbultje/ffmpeg-realmedia-rtsp/
>>> And here's another one, which makes RTPDemuxContext opaque for
>>> DynamicProtocolHandler functions. The advantage of this is that we
>>> don't rely on DynamicProtocol* being limited to using RTPDemuxContext,
>>> which would in a theoretical next patch (coming in 2 minutes)
>>
>> And that patch is here.
>
> Could you try to minimize it?

You mean split?

> --- ffmpeg-svn.orig/libavformat/rtsp.c  2008-09-30 09:13:43.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtsp.c       2008-09-30 09:13:51.000000000 -0400
> @@ -73,13 +73,13 @@
>     enum RTSPLowerTransport lower_transport;
>     enum RTSPServerType server_type;
>     char last_reply[2048]; /* XXX: allocate ? */
> -    RTPDemuxContext *cur_rtp;
> +    void *cur_rtp;
>     int need_subscription;
>  } RTSPState;
>
>  typedef struct RTSPStream {
>     URLContext *rtp_handle; /* RTP stream handle */
> -    RTPDemuxContext *rtp_ctx; /* RTP parse context */
> +    void *rtp_ctx; /* RTP parse context */
>
> why?

So, rtsp.c currently hardcodes RTPDemuxContext. My patches allow RDT
to be used in addition to RTP, and LucaA asked to not use the RTP API
in these cases. Therefore, we split it in such a way that RDT uses
rdt_*() and RDT* API, while RTP uses rtp_*() and RTP* API. rtsp.c then
decides which one to call based on the chosen transport protocol
during the SETUP.

This change means RTSP may serve either RTP or RDT, and thus we
shouldn't hardcode the type of structure of the "transport context",
or rtp_ctx. This patch makes that type opaque in rtsp.c. This is not a
problem, since rtsp.c never accesses any members of that struct,
except for one case (see below).

> Index: ffmpeg-svn/libavformat/rtp.h
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rtp.h   2008-09-30 09:06:29.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtp.h        2008-09-30 09:13:51.000000000 -0400
> @@ -24,6 +24,8 @@
>  #include "libavcodec/avcodec.h"
>  #include "avformat.h"
>
> +typedef struct RTPDynamicProtocolHandler_s RTPDynamicProtocolHandler;
> +
>
> Index: ffmpeg-svn/libavformat/rtp_internal.h
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rtp_internal.h  2008-09-30
> 09:13:49.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtp_internal.h       2008-09-30 09:13:51.000000000
> -0400
> @@ -59,7 +59,7 @@
>                                                 const uint8_t * buf,
>                                                 int len, int flags);
>
> -typedef struct RTPDynamicProtocolHandler_s {
> +struct RTPDynamicProtocolHandler_s {
>
> I'm not so sure it should go.

I think I moved it so that the new rtp_parse_set_dynamic_protocol()
function, declared in rtp.h, can be declared with
RTPDynamicProtocolHandler. Alternatively, I can make that function use
void * instead, that works too. I can also split so that I first apply
this move-of-struct-typedef and then the other parts (if you want me
to split patches, please let me know, not all of this is split to the
fullest extent possible). The function is needed to ensure that rtsp.c
doesn't access RTPDemuxContext, because that may not actually be a
RTPDemuxContext, but a RDTDemuxContext.

Ronald