[FFmpeg-devel] [PATCH] DNxHD/VC-3 encoder

[Baptiste Coudurier]

Michael Niedermayer wrote:
>>>
>>> [...]
>>>
>>>>+    ctx->m.avctx = avctx;
>>>>+    ctx->m.mb_intra = 1;
>>>>+    ctx->m.h263_aic = 1;
>>>>+
>>>>+    MPV_common_init_mmx(&ctx->m);
>>>>+    if (!ctx->m.dct_quantize)
>>>>+        ctx->m.dct_quantize = dct_quantize_c;
>>>
>>>iam an idiot, i was staring at this for maybe 2 minutes, somehow i knew
>>>it was wrong i dont understand why i didnt realize it immedeatly ...
>>>
>>>_mmx()
>>>
>>>you dont even know if the CPU is a x86 ...
>>
>>Added ifdef
> 
> 
> NO!, a *codec *muxer has no business calling any *_mmx
> think about it, what will happen with people on ppc, on arm, on sparc, ...
> also grep, no other codec does that

Hummm, in DCT_common_init in mpegvideo.c:
#if defined(HAVE_MMX)
    MPV_common_init_mmx(s);
#elif defined(ARCH_ALPHA)
    MPV_common_init_axp(s);
#elif defined(HAVE_MLIB)
    MPV_common_init_mlib(s);
#elif defined(HAVE_MMI)
    MPV_common_init_mmi(s);
#elif defined(ARCH_ARMV4L)
    MPV_common_init_armv4l(s);
#elif defined(HAVE_ALTIVEC)
    MPV_common_init_altivec(s);
#elif defined(ARCH_BFIN)
    MPV_common_init_bfin(s);
#endif

> find out which init function is the correct one and call it dont call
> architecture specific functions directly
> if theres no way to do what you want currently fix the code so it is
> possible cleanly

Yes, but it seems Im doing exactly what is done in mpegvideo.c, no ?

>>> [...]
>>>
>>>>+static int dnxhd_encode_picture(AVCodecContext *avctx, unsigned char *buf, int buf_size, void *data)
>>>>+{
>>>>+    DNXHDEncContext *ctx = avctx->priv_data;
>>>>+    AVFrame *input_frame = data;
>>>>+    int first_field = 1;
>>>>+    int offset, i, ret;
>>>>+
>>>>+    if (buf_size < ctx->cid_table->frame_size) {
>>>>+        av_log(avctx, AV_LOG_ERROR, "output buffer is too small to compress picture\n");
>>>>+        return -1;
>>>>+    }
>>>>+
>>>>+    av_picture_copy((AVPicture *)ctx->picture, data, PIX_FMT_YUV422P, avctx->width, avctx->height);
>>>
>>>why do you copy the input image?
>>
>>If picture is not allocated as 1920x1088 I need to copy it.
> 
> 
> why?

It segv in dsp->get_pixels, since last row height is not 16 but 8 (for
example when encoding raw yuv422p stream).

I tried reading last row differently but:
- it's not faster than copying and IMHO clutter the code.
- for progressive its possible to skip 4 bottom blocks (8 lines) but
interlaced mode is even more complicated since 4 lines are in field 1
and 4 are in field 2.

> [...]
> 
>>+    /* rate control */
>>+    unsigned slice_bits;
>>+    unsigned qscale;
>>+    unsigned lambda;
> 
> 
> not doxygen compatible comment
> 
> [...]

Fixed.

>>+            if (qscale < last_lower)
>>+                last_lower = qscale;
> 
> 
> FFMIN
> 

Changed and also changed set last_higher with FFMAX.

Also only do variance and qsort if needed.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: dnxhd_encoder8.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071006/e1e197e3/attachment.asc>