[vlc-devel] [vlc-commits] avcodec: fix direct rendering with some codecs

[Rafaël Carré]

Le Sun, 11 Dec 2011 21:13:07 +0100,
Laurent Aimar <fenrir at elivagar.org> a écrit :

> > > > IIUC we modify picture width so all planes are modulo 16 (IIUC), so
> > > > when there is chroma subsampling, even if the width is modulo 16, the
> > > > chroma planes are not necessary.
> > >  I don't understand this part/what you mean.
> > 
> > Usually there is the requirement that width/height are aligned on 16
> > (pixels) as required by the encoder.
> Nope, no such things.
> What avcodec requires:
>  - each pitch/stride must at least be a multiple of 16
>  - the base pointer of each plane must be aligned (16)
>  - the stride/pitch of the luma has the right ratio against the chroma (eg: *2 for 420).
>  - the buffer provided can at least store a picture of the wxh returned by
>    avcodec_align_dimensions()
>  - the pitch/stride of consecutives pictures are the sames.
> and that suppose that we do request EMU_EDGE. If we don't, they are more
> constraints that we cannot support at the moment.
> 
> > I do not fully understand the maths, I thought that we require the
> > stride of each plane to be aligned on 16 bytes, but for the sample I
> > have (720x486), the luma stride is 1472 (== 32*x) instead of 1440
> > (2 * 720 which is already aligned on 32).
> > The height is changed from 486 to 498, neither are aligned on 32; and i
> > don't understand the purpose anyway.
> 
>  Which colorspace? And what do you mean by width/height? (width/height or i_pitch/i_lines).

[0xa44a828] swscale filter debug: 720x486 chroma: I2AL -> 720x486 chroma: YUY2 with scaling using Bicubic (good quality)

So chroma is I2AL

The picture is here:
	width/height = visible width/height = 720x486
For the 3 planes though:
	width/height = 736x498
	visible width/height = 720x486

#0  ffmpeg_GetFrameBuf (p_context=0x716b20, p_ff_pic=0x717e10) at ../../../../vlc/modules/codec/avcodec/video.c:1062
1062	    p_ff_pic->age = 256*256*256*64; // FIXME FIXME from ffmpeg
(gdb) print *p_pic
$16 = {format = {i_chroma = 1279341129, i_width = 720, i_height = 486, i_x_offset = 0, i_y_offset = 0, i_visible_width = 720, 
    i_visible_height = 486, i_bits_per_pixel = 0, i_sar_num = 1, i_sar_den = 1, i_frame_rate = 2997, i_frame_rate_base = 100, 
    i_rmask = 0, i_gmask = 0, i_bmask = 0, i_rrshift = 0, i_lrshift = 0, i_rgshift = 0, i_lgshift = 0, i_rbshift = 0, 
    i_lbshift = 0, p_palette = 0x0}, p_data_orig = 0x17bc0c0, p = {{p_pixels = 0x17bc0c0 "", i_lines = 498, i_pitch = 1472, 
      i_pixel_pitch = 2, i_visible_lines = 486, i_visible_pitch = 1440}, {p_pixels = 0x186f040 "", i_lines = 498, 
      i_pitch = 736, i_pixel_pitch = 2, i_visible_lines = 486, i_visible_pitch = 720}, {p_pixels = 0x18c8800 "", 
      i_lines = 498, i_pitch = 736, i_pixel_pitch = 2, i_visible_lines = 486, i_visible_pitch = 720}, {p_pixels = 0x0, 
      i_lines = 0, i_pitch = 0, i_pixel_pitch = 0, i_visible_lines = 0, i_visible_pitch = 0}, {p_pixels = 0x0, i_lines = 0, 
      i_pitch = 0, i_pixel_pitch = 0, i_visible_lines = 0, i_visible_pitch = 0}}, i_planes = 3, i_refcount = 1, date = 0, 
  b_force = false, b_progressive = false, b_top_field_first = false, i_nb_fields = 2, p_q = 0x0, i_qstride = 0, i_qtype = 0, 
  p_sys = 0x0, pf_release = 0x7ffff7961030 <Release>, p_release_sys = 0x8e6050, p_next = 0x0}


Here is valgrind/gdb traces at invalid write


valgrind --db-attach=yes vlc --no-spu --no-osd --no-audio --no-media-library --ignore-config -I dummy -vv --ffmpeg-dr --ffmpeg-threads 1 AppleProRes422.mov 

...

[0x6a18628] avcodec decoder debug: using direct rendering
==2248== Invalid write of size 8
==2248==    at 0x145B00A8: ??? (proresdsp.asm:428)
==2248==    by 0x14453BB0: decode_slice_plane (proresdec.c:492)
==2248==    by 0x1445403F: decode_slice (proresdec.c:587)
==2248==    by 0x144D690E: avcodec_default_execute (utils.c:568)
==2248==    by 0x1445315C: decode_frame (proresdec.c:624)
==2248==    by 0x144D7277: avcodec_decode_video2 (utils.c:866)
==2248==    by 0x13EAEF70: DecodeVideo (video.c:590)
==2248==    by 0x5085966: DecoderDecodeVideo (decoder.c:1512)
==2248==    by 0x508567F: DecoderProcess (decoder.c:1878)
==2248==    by 0x508577A: DecoderThread (decoder.c:938)
==2248==    by 0x532AEFB: start_thread (pthread_create.c:304)
==2248==    by 0x582585C: clone (clone.S:112)
==2248==  Address 0x96c5650 is 0 bytes after a block of size 1,466,112 alloc'd
==2248==    at 0x4C278E8: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2248==    by 0x4C27997: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2248==    by 0x50C05AF: picture_NewFromResource (vlc_common.h:935)
==2248==    by 0x50C1267: picture_pool_NewFromFormat (picture_pool.c:138)
==2248==    by 0x50AF3DE: vout_InitWrapper (vout_wrapper.c:153)
==2248==    by 0x50A78FB: ThreadStart (video_output.c:1360)
==2248==    by 0x50A8085: Thread (video_output.c:1499)
==2248==    by 0x532AEFB: start_thread (pthread_create.c:304)
==2248==    by 0x582585C: clone (clone.S:112)
==2248== 
==2248== 


ff_prores_idct_put_10_sse4 () at libavcodec/x86/proresdsp.asm:428
428	idct_put_fn sse4, 16
(gdb) bt
#0  ff_prores_idct_put_10_sse4 () at libavcodec/x86/proresdsp.asm:428
#1  0x0000000014453bb1 in decode_slice_plane (ctx=<optimized out>, td=0x2e00, buf=<optimized out>, data_size=<optimized out>, 
    out_ptr=0x96c2290, linesize=1472, mbs_per_slice=8, blocks_per_mb=2, plane_size_factor=4, qmat=0x66e1270)
    at libavcodec/proresdec.c:492
#2  0x0000000014454040 in decode_slice (avctx=<optimized out>, td=0xc328f20) at libavcodec/proresdec.c:587
#3  0x00000000144d690f in avcodec_default_execute (c=0x66df560, func=0x14453d10 <decode_slice>, arg=<optimized out>, 
    ret=<optimized out>, count=112, size=4128) at libavcodec/utils.c:568
#4  0x000000001445315d in decode_picture (avctx=0x66df560, pic_num=1, ctx=0x66e0f20) at libavcodec/proresdec.c:624
#5  decode_frame (avctx=0x66df560, data=0x66dff60, data_size=0x8383cec, avpkt=0x8383c90) at libavcodec/proresdec.c:669
#6  0x00000000144d7278 in avcodec_decode_video2 (avctx=0x66df560, picture=0x66dff60, got_picture_ptr=0x8383cec, 
    avpkt=0x8383c90) at libavcodec/utils.c:866
#7  0x0000000013eaef71 in DecodeVideo (p_dec=0x6a18628, pp_block=<optimized out>)
    at ../../../../vlc/modules/codec/avcodec/video.c:590
#8  0x0000000005085967 in DecoderDecodeVideo (p_dec=0x6a18628, p_block=0x7291190) at ../../vlc/src/input/decoder.c:1512
#9  0x0000000005085680 in DecoderProcessVideo (b_flush=false, p_block=0x7291190, p_dec=0x6a18628)
    at ../../vlc/src/input/decoder.c:1878
#10 DecoderProcess (p_dec=0x6a18628, p_block=<optimized out>) at ../../vlc/src/input/decoder.c:2052
#11 0x000000000508577b in DecoderThread (p_data=0x6a18628) at ../../vlc/src/input/decoder.c:938
#12 0x000000000532aefc in start_thread (arg=0x8384700) at pthread_create.c:304
#13 0x000000000582585d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#14 0x0000000000000000 in ?? ()
(gdb) 

The invalid write happens when decoding V chroma plane, where we supposedly overflows the picture buffer.

> 
> > > > pointers welcome
> > >  I am a bit dubious about the fact that the proress decoder in ffmpeg doesn't
> > > seem to check the value of CODEC_FLAG_EMU_EDGE. (h264 decoder does for
> > > example).
> > 
> > I thought that perhaps edges are drawn by some helper function because
> > only a handful of decoders check this flag, or perhaps they are just
> > not drawn at all.
> > 
> > Why do we need libavcodec to draw the edges for us anyway?
>  It's not that we require the edges, it's because we cannot provide a
> buffer suitable for avcodec if we don't ask for EMU_EDGE (or if we could,
> we would probably end up doing a copy later anyway, at least for playback).
> 



-- 
Rafaël Carré