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

[Ronald S. Bultje]

Hi Luca,

On Wed, Oct 1, 2008 at 3:38 PM, Luca Abeni <lucabe72 at email.it> wrote:
> 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.

I suppose you don't like unnamed structs then, right? :-). OK, I will
go and try this. I oppose the name TransportContext, because it
suggests that it contains data related to RTP or RDT. You'll notice
that I'm not making RTPDemuxContext opaque here, but rather the access
to it. void* here isn't actually a RTPDemuxContext anymore after this
patch, but rather a datatype context (e.g. h264 or mpeg; see how the
caller changes from passing "s" to "s->dynamic_protocol_context"). I
propose the name RTPPacketDataContext, is that OK?

(I suppose I should split these two parts of the patch...)

[..] <- Michael-style, means I cut the rest of your email

Ronald