On Wed, Sep 19, 2012 at 9:22 AM, Justin Ruggles
<[email protected]> wrote:
> On 09/18/2012 10:41 PM, Nathan Caldwell wrote:
>> + /* FIXME: Opus can handle up to 255 channels. However, the mapping for
>> anything
>> + * greater than 8 is undefined.
>> + */
>> + if (avctx->channels > 8) {
>> + av_log(avctx, AV_LOG_WARNING, "Unsupported number of channels
>> (%d).\n",
>> + avctx->channels);
>> + return AVERROR(EINVAL);
>> + }
>
> This check is already covered in avcodec_open2() because you have set
> channel_layouts in the AVCodec.
I removed the return, but left a modified warning. It's relevant if we
decide to support
more than 8 channels, because of Opus' undefined channel mapping.
>> +#define OFFSET(x) offsetof(LibOpusEncContext, opts.x)
>> +#define FLAGS AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>> +static const AVOption libopus_options[] = {
>> + { "vbr", "Variable bit rate mode", OFFSET(vbr),
>> AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 2, FLAGS, "vbr" },
>> + { "off", "Use constant bit rate", 0, AV_OPT_TYPE_CONST, {
>> .i64 = 0 }, 0, 0, FLAGS, "vbr" },
>> + { "on", "Use variable bit rate", 0, AV_OPT_TYPE_CONST, {
>> .i64 = 1 }, 0, 0, FLAGS, "vbr" },
>> + { "constrained", "Use constrained VBR", 0, AV_OPT_TYPE_CONST, {
>> .i64 = 2 }, 0, 0, FLAGS, "vbr" },
>> + { "packet_loss", "Expected packet loss percentage",
>> OFFSET(packet_loss), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 100, FLAGS },
>> + { "application", "Intended application type",
>> OFFSET(application), AV_OPT_TYPE_INT, { .i64 = OPUS_APPLICATION_AUDIO },
>> OPUS_APPLICATION_VOIP, OPUS_APPLICATION_RESTRICTED_LOWDELAY, FLAGS,
>> "application" },
>> + { "voip", "Favor improved speech intelligibility", 0,
>> AV_OPT_TYPE_CONST, { .i64 = OPUS_APPLICATION_VOIP }, 0, 0,
>> FLAGS, "application" },
>> + { "audio", "Favor faithfulness to the input", 0,
>> AV_OPT_TYPE_CONST, { .i64 = OPUS_APPLICATION_AUDIO }, 0, 0,
>> FLAGS, "application" },
>> + { "lowdelay", "Favor minimum possible coding delay", 0,
>> AV_OPT_TYPE_CONST, { .i64 = OPUS_APPLICATION_RESTRICTED_LOWDELAY }, 0, 0,
>> FLAGS, "application" },
>> + { NULL },
>> +};
>
> I know there have been disagreements on formatting for AVOption lists,
> but at least some alignment would be nice...
They are aligned.
Options are all aligned with each other (except for the
default/min/max values for -application).
Named constants that all apply to the same option are indented one
level and aligned with the other constants for that option.
I've rearranged them, maybe it's better now?
--
-Nathan Caldwell
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
