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

[Luca Abeni]

Hi Ronald,

Ronald S. Bultje wrote:
[...]
> 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,
[...]

The concept is (IMHO) fine, but I have some comments on the implementation:

>  /**< return 0 on packet, no more left, 1 on packet, 1 on partial packet... */
>  static int
> -rdt_parse_packet (RTPDemuxContext *s, AVPacket *pkt, uint32_t *timestamp,
> +rdt_parse_packet (void *priv_data, AVStream *st,

I think "void *" is not a good way to have an opaque data type. Maybe we
can rename "struct rdt_data" to "struct TransportContext" or something similar,
and have rdt_parse_packet(struct TransportContext *priv_data ...).
(and in rtp_internal.h you just have "struct TransportContext;" without
actually defining the structure, that is defined in rdt.c, so it is opaque).
This will be safer and will avoid a cast.


> Index: ffmpeg-svn/libavformat/rtp_h264.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rtp_h264.c	2008-09-11 13:24:40.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtp_h264.c	2008-09-30 09:06:29.000000000 -0400
> @@ -159,14 +159,15 @@
>  }
>  
>  // return 0 on packet, no more left, 1 on packet, 1 on partial packet...
> -static int h264_handle_packet(RTPDemuxContext * s,
> +static int h264_handle_packet(void * priv_data,

As above, this can be
"static int h264_handle_packet(struct TransportContext *priv_Data...)"
(and in this case you rename h264_rtp_extra_data in "struct TransportContext").


				Luca