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

[Michael Niedermayer]

On Wed, Nov 19, 2008 at 05:00:52PM -0500, Ronald S. Bultje wrote:
> Hi Michael,
> 
> On Wed, Nov 19, 2008 at 4:21 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Tue, Nov 18, 2008 at 08:29:09PM -0500, Ronald S. Bultje wrote:
> >> @@ -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 particular RTPSStream and its contained RDTDemuxContext
> >> +     *  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]) */
> >>      AVStream *st;
> >
> > I think this design is a mess ...
> > At least st should be renamed to something like first_st_of_set
> 
> I can change stuff, just be a little bit specific about what exactly
> is a mess. Current design is sort of an evolution of me trying to get
> things to work, it's not always thought through very well, I totally
> admit that.

Well, i dont really know your / the RT*P code too well so maybe i
missunderstand something but AFAICS either
theres 1 stream per RDTDemuxContext in which case st should be that and
not the "first of a set"
or
there is a set per RDTDemuxContext, in which case the stream index and
the number of streams in the set seem to be the logic way to keep track


> 
> As a slightly different solution, I can keep the AVStream* as it is,
> so that at least these two first struct members are / can be shared
> between RDT/RTPDemuxContext (allowing for a parent struct that I can
> cast to regardless of transport). 

sharing requires them to be semantically the same,
if its the first of a set here but not for RTP then iam not sure if it
is semantically the same but again i dont know the code, iam just trying
to review because noone else is doing it ...


> I would then rename it to first_st

> (st is ok to abbreviate stream, right?)

yes


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081119/cd21ff72/attachment.pgp>