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

[Benoit Fouet]

Ronald S. Bultje a ?crit :
> Hi,
>
> On Mon, Nov 17, 2008 at 5:11 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>   
>> On Mon, Nov 17, 2008 at 09:40:34AM -0500, Ronald S. Bultje wrote:
>>     
>>> @@ -324,11 +325,12 @@
>>>      if (!s->parse_packet)
>>>          return -1;
>>>
>>> -    if (!buf) {
>>> +    if (!buf && s->prev_stream_id != -1) {
>>>          /* return the next packets, if any */
>>>          timestamp= 0; ///< Should not be used if buf is NULL, but should be set to the timestamp of the packet returned....
>>>          rv= s->parse_packet(s->dynamic_protocol_context,
>>> -                            s->st, pkt, &timestamp, NULL, 0, flags);
>>>       
>>> +                            s->ic->streams[s->st->index + s->prev_stream_id],
>>>       
>> s->st has a poor name and lacks documentation
>>     
>
> Attached patch adds docs for s->ic and s->st. The name is consistent
> with RTPDemuxContext, which I like and would like to keep this way,
> with the goal of eventually merging parts of the two so they can be
> commonly accessed without having to care about the type of context
> we're dealing with... However, as always, if it's important to you,
> I'll change it, let me know.
>
> Ronald
>   
> ------------------------------------------------------------------------
>
> Index: ffmpeg-svn/libavformat/rdt.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rdt.c	2008-11-17 17:48:19.000000000 -0500
> +++ ffmpeg-svn/libavformat/rdt.c	2008-11-17 17:54:31.000000000 -0500
> @@ -36,7 +36,12 @@
>  #include <libavcodec/bitstream.h>
>  
>  struct RDTDemuxContext {
> -    AVFormatContext *ic;
> +    AVFormatContext *ic; /**< the containing (RTSP) demux context */
> +    /**< stream context of the first stream of the set of streams that
>   

this should not have the '<', if, as I understood it, it is a
description of st

> +     *   this particular RTPSStream and its RTP parsing context (this one)
>   

why are you adding '(this one)' ?

> +     *   are describing. In case of multiple streams per set, the rest
> +     *   of the streams are directly following in the streams array
> +     *   of the 'ic' variable (e.g. me->ic->streams[me->st->index + 1]) */
>   

that does not sound very good to me, though understandable, but I cannot
find another way of saying it right now...

>      AVStream *st;
>      void *dynamic_protocol_context;
>      DynamicPayloadPacketHandlerProc parse_packet;
>