On Mon, Feb 26, 2018 at 06:10:42AM +0100, Luca Barbato wrote:
> ---
>
> Since the bitstream will be frozen soon shouldn't hurt adding it.
I'd prefer if you waited until it is actually frozen.
> configure | 1 +
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 2 +-
> libavcodec/libaomenc.c | 602
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 605 insertions(+), 1 deletion(-)
docs, changelog
> --- /dev/null
> +++ b/libavcodec/libaomenc.c
> @@ -0,0 +1,602 @@
> +
> +#include "avcodec.h"
> +#include "internal.h"
> +#include "libaom.h"
> +#include "libavutil/base64.h"
> +#include "libavutil/common.h"
> +#include "libavutil/mathematics.h"
> +#include "libavutil/opt.h"
canonical header ordering please
> +static av_cold void dump_enc_cfg(AVCodecContext *avctx,
> + const struct aom_codec_enc_cfg *cfg)
> +{
> + int width = -30;
> + int level = AV_LOG_DEBUG;
> +
> + av_log(avctx, level, "aom_codec_enc_cfg\n");
> + av_log(avctx, level, "generic settings\n"
> + " %*s%u\n %*s%u\n %*s%u\n %*s%u\n %*s%u\n"
> + " %*s{%u/%u}\n %*s%u\n %*s%d\n %*s%u\n",
> + width, "g_usage:", cfg->g_usage,
Looks like pointless indirection to me.
> +static av_cold int codecctl_int(AVCodecContext *avctx,
> + enum aome_enc_control_id id, int val)
> +{
> + AOMContext *ctx = avctx->priv_data;
> + char buf[80];
> + int width = -30;
> + int res;
> +
> + snprintf(buf, sizeof(buf), "%s:", ctlidstr[id]);
> + av_log(avctx, AV_LOG_DEBUG, " %*s%d\n", width, buf, val);
The format string does not match the number of arguments.
> + res = aom_codec_control(&ctx->encoder, id, val);
> + if (res != AOM_CODEC_OK) {
> + snprintf(buf, sizeof(buf), "Failed to set %s codec control",
> + ctlidstr[id]);
> + log_encoder_error(avctx, buf);
> + }
> +
> + return res == AOM_CODEC_OK ? 0 : AVERROR(EINVAL);
I think it's clearer to return from the if above instead of using the
ternary operator here.
> +static av_cold int aom_init(AVCodecContext *avctx)
> +{
> + else
> + enccfg.rc_target_bitrate = av_rescale_rnd(avctx->bit_rate, 1, 1000,
> + AV_ROUND_NEAR_INF);
Indentation is off.
> + //0-100 (0 => CBR, 100 => VBR)
> + //_enc_init() will balk if kf_min_dist differs from max w/AOM_KF_AUTO
nit: space after //
> + ctx->twopass_stats.sz = strlen(avctx->stats_in) * 3 / 4;
nit: stray double space before =
> + //codec control failures are currently treated only as warnings
> + //provide dummy value to initialize wrapper, values will be updated each
> _encode()
nit: space after //
> + * Store coded frame information in format suitable for return from
> encode2().
in a format
> +static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
> +{
> + memcpy((uint8_t*)stats->buf + stats->sz,
(uint8_t *)
> + case AOM_CODEC_PSNR_PKT: //FIXME add support for AV_CODEC_FLAG_PSNR
> + case AOM_CODEC_CUSTOM_PKT:
> + //ignore unsupported/unrecognized packet types
nit: space after //
> +static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
> + const AVFrame *frame, int *got_packet)
> +{
> + if (!frame && avctx->flags & AV_CODEC_FLAG_PASS1) {
> + unsigned int b64_size = AV_BASE64_SIZE(ctx->twopass_stats.sz);
> +
> + avctx->stats_out = av_malloc(b64_size);
> + if (!avctx->stats_out) {
> + av_log(avctx, AV_LOG_ERROR, "Stat buffer alloc (%d bytes)
> failed\n",
> + b64_size);
%u bytes, or better yet use size_t and %zu
> + { "partitions", "The frame partitions are independently decodable "
> + "by the bool decoder, meaning that partitions can
> be decoded even "
> + "though earlier partitions have been lost. Note
> that intra predicition"
predic_tion
Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
