[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