[Ffmpeg-devel] [PATCH] avcodec_open Doxygen docs

[Michael Niedermayer]

Hi

On Thu, Feb 22, 2007 at 03:02:49AM +0100, Panagiotis Issaris wrote:
[...]
> >>>[...]
> >>>      
> >>Size of the output buffer?
> >>    
> >>>and the 32/64 input padding applies to audio too!
> >>> 
> >>>      
> >>Fixed.
> >>
> >>Does the output and input buffer also have to be aligned on a 16 byte 
> >>boundary for avcodec_decode_video()?
> >>    
> >
> >the true alignment requirements depend for every buffer on the cpu that
> >is if the cpu has no alignment requirements then it all might work fine
> >with no alignment at all though maybe slower (that again depends on the 
> >cpu)
> >
> >in practice though the bitstream should have 4byte alignment at minimum and
> >all sample data be it audio or video should be 16byte aligned unless the
> >cpu doesnt need it (altivec/sse do) that also means if the linesize is not
> >a multiple of 16 then theres no sense in aligning the buffer start to 16
> >  
> I've added a note to both avcodec_decode_video() and 
> avcodec_decode_audio() saying:
> "You might have to align the input buffer \p buf and output buffer \p
> samples. The alignment requirements depend on the CPU: on some CPUs it isn't
> necessary at all, on others it won't work at all if not aligned and on 
> others
> it will work but it will have an impact on performance. In practice, the
> bitstream should have 4 byte alignment at minimum and all sample data should
> be 16 byte aligned unless the CPU doesn't need it (AltiVec and SSE do). If
> the linesize is not a multiple of 16 then there's no sense in aligning the
> start of the buffer to 16."
> 
> Does that sound alright?

yes


[...]
> >[...]
> >  
> >>+/**
> >>+ * Checks if the given dimension of a picture is valid.
> >>+ *
> >>    
> >
> >at this point noone will have learnt what the function does, what is 
> >"valid"
> >
> >because 80000x80000 is valid in some sense ...
> >  
> Hmm. You are right...
> 
> Would something like this be better? Or should I explicitly mention the 
> checks?
> 
> 2578  * Checks if the given dimension of a picture is valid, meaning 
> that width and
> 2579  * height are sane and the total picture size is allocatable.
> 
> I'm not sure actually about the (w+128)*(uint64_t)(h+128) < INT_MAX/4) 
> part of the
> check: For 64-bit machines, couldn't this check be too strict? IOW isn't 
> INT_MAX
> still somewhere near 2^31 on 64-bit machines?

INT_MAX can be INT64_MAX depending on compiler but more important is that
the whole code needs to be audited if INT_MAX where changed to LONG_MAX


> 
> or:
> 2578  * Checks if the given dimension of a picture is valid, meaning 
> that width and
> 2579  * height are sane and the total picture size is addressable.
> 
> or:
> 2578  * Checks if the given dimension of a picture is valid, meaning 
> that width and
> 2579  * height and total picture size are sane.
> 
> And preference? Or a better suggestion maybe?

maybe somethng like

Checks if the given dimension of a picture is valid, meaning that all
bytes of the picture can be addressed with a signed int



[...]
> >  
> >>+ * @warning The end of the input buffer \p buf should be set to 0 to 
> >>ensure that
> >>+ * no overreading happens for damaged MPEG streams.
> >>    
> >
> >this trick is limited to video i _think_ though of course it does no harm
> >for audio
> >  
> Ah, apiexample.c used it for audio decoding too, which is why I added 
> it. Attached patch removes it in case anyone
> confirms to be 100% sure it does no harm to remove it.

apply it

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070222/443662c9/attachment.pgp>