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

[Baptiste Coudurier]

Michael Niedermayer wrote:
> Hi
> 
> On Sat, Oct 06, 2007 at 02:10:52AM +0200, Baptiste Coudurier wrote:
> 
>>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 ?
> 
> 
> that code is not specific to a single codec, or do you see it in msmpeg4.c
> in h263.c, ... ?
> also you copied only the _mmx (and if you would copy all it would be code
> duplication)
> 

Yes I saw that after, using DCT_common_init now.

>>>>>[...]
>>>>>
>>>>>
>>>>>>+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.
> 
> 
> so there are X lines at th bottom and you read more than X and that segfaults
> and as "solution" you copy the whole 1920x1080 image
> i really do think you should not read lines after the image
> 

I changed get_blocks to read only what's needed though it's not faster,
please don't say dnxhd_get_pixels_4x8 is duplicated...

I'll submit patches to rename functions.

-- 
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_encoder9.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071006/6e77d2c8/attachment.txt>