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

[Cyril Russo]

Michael Niedermayer a ?crit :
> On Mon, May 18, 2009 at 02:11:02PM +0200, Cyril Russo wrote:
>   
>> Hi,
>>
>> Ping for previous mail. I've made the correction asked by Michael in the 
>> attached patch. Please review.
>>     
>
> [...]
>   
>> +/** Translate an FFmpeg Picture into its VA API form */
>> +static void vaapi_h264_fill_vaapi_picture(VAPictureH264 *va_pic, Picture *pic, int pic_structure)
>>     
>
> Missing documentation for the function parameters
>   
Done (and for other complex functions too)
> [...]
>   
>> +/** Initialize reference picture lists (7.4.3.1) */
>> +static void vaapi_h264_fill_RefPicList(VAPictureH264 RefPicList[32], Picture *ref_list, unsigned int ref_count)
>>     
>
> 7.4.3.1 ?
> from h264 ? from vaapi?
>
> and if its h264, what is that supposed to mean? the file is a interface
> between vaapi and our h264 decoder.
>
>   
Well, in fact it's simply refering to ITU H264 reference picture list 
reordering semantics (7.4.3.1) that VA API requires.
(And same for the Prediction weight table semantics (7.4.3.2) below).
VA API is more strict (dumb ?) about those and requires default and 
inferred values to be included in its buffer too (it doesn't compute 
them at runtime like ffmpeg's H264 does).

As I understand your remark, I'm removing the comment (and adding 
@param, remark #1) from the function's comment,
and move a comment about the used algorithm to the function body (so any 
later maintainer will understand the algorithm used).

> [...]
>   
>> +static int vaapi_h264_start_frame(AVCodecContext *avctx, av_unused const uint8_t *buffer, av_unused uint32_t size)
>>     
>
> i would prefer all vaapi prefixes to be
> removed from function names that are part of libav*, such prefix should
> be reserved for vaapi
>   
Even if this function is only used in VAAPI, for VAAPI ?
If I understand correctly, please review the attached patch.

> PS: also any other kind of cleanup you can think of is likely welcome
>   
Cosmetics or code size ?
I've fixed the doxystuff and made sure everything was properly aligned.
Concerning the code size, the current patch is very orthogonal (there is 
not many things I could do to reduce its footprint, unless making very 
h4ckR obfuscation).

Let me know if this version better fit.
>
> [...]
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ffmpeg.hwaccel.vaapi.h264.11.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090520/127ad128/attachment.asc>