[FFmpeg-devel] Apple Prores Encoder

Wasserman Anatoliy anatoliy.wasserman at yandex.ua
Sat Oct 29 03:40:12 CEST 2011


Jean,

Thank you for your review, I've incorporated your comments into the code.
The new patch is attached.

Anatoliy

28.10.2011, 17:34, "Jean First" <jeanfirst at gmail.com>:
> On Fri Oct 28 2011 22:43:57 GMT+0200 (CEST), Wasserman Anatoliy wrote:
>
>>  Hello,
>>
>>  I have developed Apple Prores Encoder.
>
> Nice.
>
>>  It has rate-control for 4 profiles: 'apch' - 185mbps, 'apcn' - 112mbps, 'apcs' - 75mbps, 'apco' - 36mbps.
>>  The profiles are triggered from ffmpeg command line via '-profile' option ( 0 - apco, 1 - apcs, 2 - apcn, 3 - apch), the default profile is apch.
>>  It accepts yuv422p10le pix format frames.
>>  It's not yet multithreaded.
>
> ...
>
>>  +- Prores encoder
>
> Please add it to the end of version next:
> Entries are sorted chronologically from oldest to youngest within each
> release.
>
>>  diff --git a/libavcodec/proresenc.c b/libavcodec/proresenc.c
>>  new file mode 100644
>>  index 0000000..42df560
>>  --- /dev/null
>>  +++ b/libavcodec/proresenc.c
>>  @@ -0,0 +1,558 @@
>>  +/*
>>  + * Apple ProRes encoder
>>  + *
>>  + * Copyright (c) 2010-2011 Anatoliy Wasserman
>
> 2010 ? are you sure ?
>
>>  +static void encode_codeword(PutBitContext *pb, int val, int codebook) {
>
> K&R puts the { for a function on the next line. (also below)
>
>>  +    unsigned int rice_order, exp_order, switch_bits, first_exp, exp,
>>  zeros, mask;
>>  +
>>  +    /* number of bits to switch between rice and exp golomb */
>>  +    switch_bits = codebook & 3;
>>  +    rice_order = codebook >> 5;
>>  +    exp_order = (codebook >> 2) & 7;
>
> align to the = if possible (also below)
>
>>  +#define TO_GOLUMB(val) ((val << 1) ^ (val >> 31))
>
> TO_GOLOMB ?
>
>>  +#define TO_GOLUMB2(val,sign) (val==0 ? 0 : (val << 1) + sign)
>
> TO_GOLOMB2 ?
>
>>  +static void encode_dc_coeffs(PutBitContext *pb, DCTELEM *in,
>>  +                                              int blocks_per_slice,
>>  int *qmat)
>>  +{
>>  +    int prev_dc, code;
>>  +    int i, sign, idx;
>>  +    int new_dc, delta, diff_sign, new_code;
>>  +
>>  +    prev_dc = QSCALE(qmat, 0, in[0] - 16384);
>>  +    code = TO_GOLUMB(prev_dc);
>>  +    encode_codeword(pb, code, FIRST_DC_CB);
>>  +
>>  +
>>  +    code = 5; sign = 0; idx = 64;
>
> define sign and idx to the beginning oft he function
>
>>  +    for (i = 1; i < blocks_per_slice; i++, idx += 64) {
>>  +        new_dc = QSCALE(qmat, 0, in[idx] - 16384);
>>  +        delta = new_dc - prev_dc;
>>  +        diff_sign = DIFF_SIGN(delta, sign);
>>  +        new_code = TO_GOLUMB2(get_level(delta), diff_sign);
>>  +        encode_codeword(pb, new_code, dc_codebook[FFMIN(code, 6)]);
>>  +        code = new_code;
>>  +        sign = delta >> 31;
>>  +        prev_dc = new_dc;
>>  +    }
>>  +}
>
> align the = (also below)
>
>>  +static av_always_inline unsigned encode_slice_data(AVCodecContext
>>  *avctx, uint8_t *dest_y, uint8_t *dest_u, uint8_t *dest_v,
>>  +        int luma_stride, int chroma_stride, unsigned mb_count,
>>  uint8_t *buf, unsigned data_size, unsigned* y_data_size,
>>  +        unsigned* u_data_size, unsigned* v_data_size, int qp)
>>  +{
>>  +    ProresContext* ctx = (ProresContext*) avctx->priv_data;
>>  +    *y_data_size = encode_slice_plane(avctx, mb_count, dest_y,
>>  luma_stride, buf, data_size, ctx->qmat_luma[qp - 1], 0);
>>  +    if (!(avctx->flags & CODEC_FLAG_GRAY)) {
>>  +        *u_data_size = encode_slice_plane(avctx, mb_count, dest_u,
>>  chroma_stride, buf + *y_data_size,
>>  +                data_size - *y_data_size, ctx->qmat_chroma[qp - 1], 1);
>>  +        *v_data_size = encode_slice_plane(avctx, mb_count, dest_v,
>>  chroma_stride, buf + *y_data_size + *u_data_size,
>>  +                data_size - *y_data_size - *u_data_size,
>>  ctx->qmat_chroma[qp - 1], 1);
>>  +    }
>>  +    return *y_data_size + *u_data_size + *v_data_size;
>>  +}
>
> very long lines and no spacing.
>
>>  +static int encode_slice(AVCodecContext *avctx, AVFrame *pic, int
>>  mb_x, int mb_y, unsigned mb_count, uint8_t *buf,
>>  +        unsigned data_size, int unsafe, int *qp)
>>  +{
>>  +    int luma_stride, chroma_stride;
>>  +    int hdr_size = 6, slice_size;
>>  +    uint8_t *dest_y, *dest_u, *dest_v;
>>  +    unsigned y_data_size = 0, u_data_size = 0, v_data_size = 0;
>>  +    ProresContext* ctx = (ProresContext*)avctx->priv_data;
>>  +    int tgt_bits = (mb_count * bitrate_table[avctx->profile]) >> 2;
>>  +    int low_bytes = (tgt_bits - (tgt_bits >> 3)) >> 3; // 12% bitrate
>>  fluctuation
>>  +    int high_bytes = (tgt_bits + (tgt_bits >> 3)) >> 3;
>>  +
>
> align the =
>
>>  +    luma_stride = pic->linesize[0];
>>  +    chroma_stride = pic->linesize[1];
>>  +
>>  +    dest_y = pic->data[0] + (mb_y << 4) * luma_stride + (mb_x << 5);
>>  +    dest_u = pic->data[1] + (mb_y << 4) * chroma_stride + (mb_x << 4);
>>  +    dest_v = pic->data[2] + (mb_y << 4) * chroma_stride + (mb_x << 4);
>
> nit: align the +
>
>>  +        subimage_with_fill((uint16_t *) pic->data[0], mb_x << 4, mb_y
>>  << 4, luma_stride, avctx->width, avctx->height,
>>  +                (uint16_t *) ctx->fill_y, mb_count << 4, 16);
>>  +        subimage_with_fill((uint16_t *) pic->data[1], mb_x << 3, mb_y
>>  << 4, chroma_stride, avctx->width >> 1,
>>  +                avctx->height, (uint16_t *) ctx->fill_u, mb_count <<
>>  3, 16);
>>  +        subimage_with_fill((uint16_t *) pic->data[2], mb_x << 3, mb_y
>>  << 4, chroma_stride, avctx->width >> 1,
>>  +                avctx->height, (uint16_t *) ctx->fill_v, mb_count <<
>>  3, 16);
>>  +        encode_slice_data(avctx, ctx->fill_y, ctx->fill_u,
>>  ctx->fill_v, mb_count << 5, mb_count << 4, mb_count,
>>  +                buf + hdr_size, data_size - hdr_size, &y_data_size,
>>  &u_data_size, &v_data_size, *qp);
>>  +    } else {
>>  +        slice_size = encode_slice_data(avctx, dest_y, dest_u, dest_v,
>>  luma_stride, chroma_stride, mb_count,
>>  +                buf + hdr_size, data_size - hdr_size, &y_data_size,
>>  &u_data_size, &v_data_size, *qp);
>>  +        if (slice_size > high_bytes && *qp <
>>  qp_end_table[avctx->profile]) {
>>  +            do {
>>  +                *qp += 1;
>>  +                slice_size = encode_slice_data(avctx, dest_y, dest_u,
>>  dest_v, luma_stride, chroma_stride, mb_count,
>>  +                        buf + hdr_size, data_size - hdr_size,
>>  &y_data_size, &u_data_size, &v_data_size, *qp);
>>  +            } while (slice_size > high_bytes && *qp <
>>  qp_end_table[avctx->profile]);
>>  +        } else if (slice_size < low_bytes && *qp >
>>  qp_start_table[avctx->profile]) {
>>  +            do {
>>  +                *qp -= 1;
>>  +                slice_size = encode_slice_data(avctx, dest_y, dest_u,
>>  dest_v, luma_stride, chroma_stride, mb_count,
>>  +                        buf + hdr_size, data_size - hdr_size,
>>  &y_data_size, &u_data_size, &v_data_size, *qp);
>>  +            } while (slice_size < low_bytes && *qp >
>>  qp_start_table[avctx->profile]);
>>  +        }
>>  +    }
>>  +
>
> alignement and spacing
>
>>  +
>>  +static int prores_encode_picture(AVCodecContext *avctx, AVFrame *pic,
>>  uint8_t *buf, const int buf_size)
>>  +{
>>  +    int mb_width = (avctx->width + 15) >> 4;
>>  +    int mb_height = (avctx->height + 15) >> 4;
>>  +    int hdr_size, sl_size, *slice_sizes;
>>  +    int sl, mb_y, sl_data_size, qp;
>>  +    int unsafe_bot, unsafe_right;
>>  +    uint8_t *sl_data;
>>  +
>>  +    int slice_per_line = 0, rem = mb_width;
>>  +    for (int i = av_log2(DEFAULT_SLICE_MB_WIDTH); i >= 0; --i) {
>>  +        slice_per_line += rem >> i;
>>  +        rem &= (1 << i) - 1;
>>  +    }
>>  +
>>  +    qp = qp_start_table[avctx->profile];
>>  +    slice_sizes = av_malloc(slice_per_line * mb_height * sizeof(int));
>>  +    sl = 0; hdr_size = 8; sl_data_size = buf_size - hdr_size;
>>  +    sl_data = buf + hdr_size + (slice_per_line * mb_height * 2);
>
> put the local variables at the beginning of the function
>
>>  +        av_log(avctx, AV_LOG_ERROR, "prores: need YUV422P10\n");
>
> I think you can drp the "prores:" part in the log message here and below
>
>>  +        return -1;
>>  +    }
>>  +    if (avctx->width & 0x1) {
>>  +        av_log(avctx, AV_LOG_ERROR, "prores: frame width needs to be
>>  multiple of 2\n");
>>  +        return -1;
>>  +    }
>>  +
>
> is an odd height allowed ?
>
>>  +    memset(ctx, 0, sizeof(ProresContext));
>>  +    if ((avctx->height & 0xf) || (avctx->width & 0xf)) {
>>  +        ctx->fill_y = av_malloc(DEFAULT_SLICE_MB_WIDTH << 9);
>>  +        ctx->fill_u = av_malloc(DEFAULT_SLICE_MB_WIDTH << 8);
>>  +        ctx->fill_v = av_malloc(DEFAULT_SLICE_MB_WIDTH << 8);
>>  +    }
>>  +    if (avctx->profile == FF_PROFILE_UNKNOWN)
>>  +        avctx->profile = FF_PROFILE_PRORES_HQ;
>
> maybe add:
> av_log(avctx, AV_LOG_INFO, "encoding with ProRes HQ (apch) profile\n",
>
>>  +    else if (avctx->profile < FF_PROFILE_PRORES_PROXY ||
>>  avctx->profile > FF_PROFILE_PRORES_HQ) {
>>  +        av_log(avctx, AV_LOG_ERROR, "prores: unknown profile %d, use
>>  [0 - apco, 1 - apcs, 2 - apcn, 3 - apch]\n",
>>  +                avctx->profile);
>
> maybe: ..., 3 - apch (default)]\n
>
>>  +        return -1;
>>  +    }
>>  +    avctx->codec_tag = *((const int*)profiles[avctx->profile].name);
>>  +
>>  +    for(i = 1; i <= 16; i++) {
>
> nit: for (
>
>>  +        scale_mat(QMAT_LUMA[avctx->profile], ctx->qmat_luma[i - 1], i);
>>  +        scale_mat(QMAT_CHROMA[avctx->profile], ctx->qmat_chroma[i -
>>  1], i);
>
> nit: align the commas
>
>>  +    .pix_fmts= (const enum PixelFormat[]){PIX_FMT_YUV422P10LE,
>>  PIX_FMT_NONE},
>
> align the =
>
>>  +    .long_name      = NULL_IF_CONFIG_SMALL("Apple ProRes 422"),
>
> There is also an 4444 variant - maybe "Apple ProRes" is enough
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Apple-ProRes-encoder.patch
Type: application/octet-stream
Size: 24027 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111028/d0e63b6f/attachment.obj>


More information about the ffmpeg-devel mailing list