[FFmpeg-devel] [PATCH][VAAPI][6/6] Add H.264 bitstream decoding (take 11)

Diego Biurrun diego
Wed May 20 12:13:37 CEST 2009


On Wed, May 20, 2009 at 10:06:32AM +0200, Cyril Russo wrote:
> 
> --- libavcodec/vaapi_h264.c	(r??vision 0)
> +++ libavcodec/vaapi_h264.c	(r??vision 0)
> @@ -0,0 +1,338 @@
> +
> +/** Translate an FFmpeg Picture into its VA API form. 
> +    @param[out] va_pic          A pointer on a lib VA's own picture struct

A pointer is never "on" but "to" something, more instances below.

> +    @param[in]  pic             A pointer the ffmpeg picture struct to convert

pointer to?

> +static void vaapi_h264_fill_vaapi_picture(VAPictureH264 * va_pic, 
> +                                          Picture *       pic, 
> +                                          int             pic_structure)

The * placement is inconsistent with the rest of the code, same all over.

> +/** Append Picture into the decoded picture buffer, in a VA API form so that to 
> +    merge the second field picture attributes, if any.

"so that to merge" is not English and I am not sure what you mean here.

> +    The decodec picture buffer's size must be large enough 
> +    to receive the new VA API picture object */

End sentences in periods.

> +/** Initialize VAPictureParameterBufferH264.ReferenceFrames[] array
> +    from its ffmpeg counterpart. */

nit: I think all the Doxygen comments would look nicer with * at the
left, it's what we do in the rest of FFmpeg.

> +static int vaapi_h264_fill_ReferenceFrames(VAPictureParameterBufferH264 *pic_param, H264Context *h)

nit: long line

> +/** Initialize VA API reference picture lists from ffmpeg reference picture list.

from the

Please capitalize FFmpeg properly unless you are referrring to the
ffmpeg binary, same in other places.

> +    @param[out] RefPicList  The lib VA's internal reference picture list

What is "the lib VA"?

> +    @param[out] luma_weight_flag    Set to the luma weight flags used
> +    @param[out] luma_weight         The final luma weight to use
> +    @param[out] luma_offset         The final luma offset to use 
> +    @param[out] chroma_weight_flag  Set to the chroma weight flags used 
> +    @param[out] chroma_weight       The final chroma weight to use
> +    @param[out] chroma_offset       The final chroma offset to use */

Why do you use "Set to" in the explanation of a few parameters?  Seems
unnecessary..

> +/** End an hardware decoding based frame. */

a hardware

> +/** Decode the given H264 slice with VAAPI. */

H.264

Diego



More information about the ffmpeg-devel mailing list