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

[Michael Niedermayer]

On Fri, Jan 04, 2008 at 08:53:16AM -0500, Ronald S. Bultje wrote:
> 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.

patch ok

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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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/20080105/47caa778/attachment.pgp>