[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