[FFmpeg-devel] [PATCH] RTP/Vorbis payload implementation (GSoC qual task)

[Michael Niedermayer]

On Sun, Apr 12, 2009 at 01:48:26PM +0100, Colin McQuillan wrote:
> Thanks for the reviews! Updated patch attached.
> 
> 2009/4/12 Luca Barbato <lu_zero at gentoo.org>:
> [...]
> >
> > +/**
> > + * Length encoding described in RFC5215 section 3.1.1.
> > + */
> > +static int read_base128(uint8_t ** buf, uint8_t * buf_end)
> > +{
> > +    int n = 0;
> > +    for (; *buf < buf_end; ++*buf) {
> > +        n <<= 7;
> > +        n += **buf & 0x7f;
> > +        if (!(**buf & 0x80)) {
> > +            ++*buf;
> > +            return n;
> > +        }
> > +    }
> > +    return 0;
> > +}
> >
> > You should use the implementation that's already present in ffmpeg.
> >
> 
> There's a ByteIOContext function ff_get_v but I couldn't find anything
> for plain bytes (I grepped for <<=\?\s*7 and \*\s*128).
> 
> 2009/4/12 Michael Niedermayer <michaelni at gmx.at>:
> [...]
> 
> >
> >> +
> >> + ? ? ? ?/* loop on each attribute */
> >> + ? ? ? ?while (rtsp_next_attr_and_value
> >> + ? ? ? ? ? ? ? (&p, attr, sizeof(attr), value, value_alloc)) {
> >> + ? ? ? ? ? ?/* grab the codec extra_data from the config parameter of the fmtp line */
> >> + ? ? ? ? ? ?sdp_parse_fmtp_config_vorbis(stream, vorbis_data, attr, value);
> >> + ? ? ? ?}
> >> + ? ? ? ?av_free(value);
> >
> > also duplicate to at least rtp_h264.c
> >
> [...]
> 
> The while loop is common to both. Would you suggest a

the whole fmtp parsing looks common


[...]
> +/**
> + * Out-of-band headers, described in RFC 5251 section 3.2.1
> + */
> +static unsigned int
> +parse_packed_headers(const uint8_t * packed_headers,
> +                     const uint8_t * packed_headers_end,
> +                     AVCodecContext * codec, PayloadContext * vorbis_data)
> +{
> +    unsigned num_packed, num_headers, length, length1, length2;
> +    uint8_t *ptr;
> +

> +    num_packed = bytestream_get_be32(&packed_headers);
> +    vorbis_data->ident = bytestream_get_be24(&packed_headers);
> +    length = bytestream_get_be16(&packed_headers);

vertical align


> +    num_headers = get_base128(&packed_headers, packed_headers_end);
> +    length1 = get_base128(&packed_headers, packed_headers_end);
> +    length2 = get_base128(&packed_headers, packed_headers_end);

same


[...]
> +    ptr = codec->extradata = av_mallocz(length + length / 255 + 64);
> +    if (!ptr) {
> +        av_log(codec, AV_LOG_ERROR, "Out of memory");
> +        return -1;

AVERROR(ENOMEM)

[...]
-- 
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/20090412/534ab43d/attachment.pgp>