[FFmpeg-devel] [PATCH 1/3] oggdec: add support for Opus codec.

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Jul 10 22:04:58 CEST 2012


On Wed, Jul 04, 2012 at 05:28:19PM +0200, Nicolas George wrote:
> +        priv = os->private = av_mallocz(sizeof(struct oggopus_private));

sizeof(*priv) I'd suggest.

> +        if (os->psize < OPUS_HEAD_SIZE || AV_RL8(packet + 8) != 1)
> +            return AVERROR_INVALIDDATA;
> +        st->codec->codec_type  = AVMEDIA_TYPE_AUDIO;
> +        st->codec->codec_id    = CODEC_ID_OPUS;
> +        st->codec->channels    = AV_RL8 (packet + 9);
> +        /*pre_skip             = AV_RL16(packet + 10);*/
> +        /*orig_sample_rate     = AV_RL32(packet + 12);*/
> +        /*gain                 = AV_RL16(packet + 16);*/
> +        /*channel_map          = AV_RL8 (packet + 18);*/

using AV_RL8 seems a bit like overkill.
Or maybe it's worth going all the way and use the bytestream functions?

> +        priv->need_comments--;

Why decrement? Can it become > 1 somehow?

> +    toc = *packet;
> +    toc_config = toc >> 2;
> +    frame_size = toc_config < 12 ? FFMAX(480, 960 * (toc_config & 3)) :
> +                 toc_config < 16 ? 480 << (toc_config & 1) :
> +                                   120 << (toc_config & 3);

Is there any system to this? Looks rather ugly...

> +    if ((toc & 3) == 3) {
> +        if (os->psize < 2)
> +            return AVERROR_INVALIDDATA;
> +        nb_frames = packet[1] & 0x3F;
> +    } else if ((toc & 3)) {
> +        nb_frames = 2;
> +    }

The & 3 could be factored out I'd say.

> +    os->pduration = frame_size * nb_frames;
> +    /*if (os->lastpts != AV_NOPTS_VALUE) {
> +        if (st->start_time == AV_NOPTS_VALUE)
> +            st->start_time = os->lastpts;
> +        os->lastdts = os->lastpts -= st->codec->skip_data;
> +    }*/

I don't think comment-out code should be there.

> +    return 0;
> +}
> +
> +const struct ogg_codec ff_opus_codec = {
> +    .name      = "Opus",
> +    .magic     = "OpusHead",
> +    .magicsize = 8,
> +    .header    = opus_header,
> +    .packet    = opus_packet,
> +};
> +
> -- 
> 1.7.10
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list