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

[Ronald S. Bultje]

Hi,

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.

Ronald
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rtp-always_use_vfunc-parse_packet.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080104/00b7f454/attachment.asc>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rtp-always_use_vfunc-parse_packet-reindent.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080104/00b7f454/attachment.txt>