[FFmpeg-devel] [PATCH] Realmedia / RTSP (RDT)

[Ronald S. Bultje]

Hi,

On Jan 5, 2008 4:27 PM, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Fri, Jan 04, 2008 at 08:53:16AM -0500, Ronald S. Bultje wrote:
> > On Jan 2, 2008 11:04 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > On Tue, Jan 01, 2008 at 02:36:35PM -0500, Ronald S. Bultje wrote:
> > > > On Dec 29, 2007 7:43 PM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> > > > > > -                av_new_packet(pkt, len);
> > > > > > -                memcpy(pkt->data, buf, len);
> > > > > > -            }
> > > > > > +            av_new_packet(pkt, len);
> > > > > > +            memcpy(pkt->data, buf, len);
> > > > >
> > > > > cosmetic
> > > >
> > > >
> > > > Fixed. New version attached without the above. I attach a second
> patch
> > > > (sorry :-) ) which does the reindent separately (it's ok to do
> reindent
> > > > patches in the same reply, because they don't really do anything,
> > > right?).
> > > [...]
> > > > @@ -671,6 +671,8 @@
> > > >              s->read_buf_index = 0;
> > > >              return 1;
> > > >          }
> > > > +    } else if (s->parse_packet) {
> > > > +        rv= s->parse_packet(s, pkt, &timestamp, buf, len, flags);
> > > >      } else {
> > > >          // at this point, the RTP header has been stripped;  This
> is
> > > ASSUMING that there is only 1 CSRC, which in't wise.
> > > >          switch(st->codec->codec_id) {
> > > > @@ -726,12 +728,8 @@
> > > >              rv= 0;
> > > >              break;
> > > >          default:
> > > > -            if(s->parse_packet) {
> > > > -                rv= s->parse_packet(s, pkt, &timestamp, buf, len);
> > > > -            } else {
> > > > -                av_new_packet(pkt, len);
> > > > -                memcpy(pkt->data, buf, len);
> > > > -            }
> > > > +            av_new_packet(pkt, len);
> > > > +            memcpy(pkt->data, buf, len);
> > > >              break;
> > > >          }
> > >
> > > now, this part looks like a unrelated simplification, i think its ok
> > > (rtp maintainer?) but i think it should be in a seperate patch
> > > and without the reindention
> >
> >
> > You're right, it should be separate. It's actually an important
> refactoring.
> > In the current behaviour, rtp_parse_packet() will always use its own
> packet
> > parsing (e.g. for MPEG, AAC, MP3, etc.) except if the codec is unknown.
> In
> > that case, it uses either the parse_packet() vfunc in the dynamic packet
> > handler or it plainly copies the data.
> >
> > With this patch, it will always use the vfunc in the dynamic packet
> handler,
> > even if the codec is known. This is important for RDT, because RDT isn't
> an
> > actual codec, but is more of a special kind of RTP packeting. In the old
> > code (w/o this patch), something like AAC inside RDT would be parsed
> with
> > the default RTP packet layout, which would fail (because RDT/AAC is not
> the
> > same as AAC). With this patch, we can set the vfunc to the ones in
> rtp_rm.c
> > (which rtsp.c does automatically after reading the mimetype in the SDP),
> and
> > then even if the codec is AAC (which is what many Real servers serve
> > nowadays), it will use RDT packet parsing, which will make it work.
> >
> > I think it's conceptually correct to always use the vfunc if it is
> there,
> > which is why I think it should be committed separately of the rest of
> the
> > RDT patch.
>
> patch ok


Applied.

Ronald