[Libva] [PATCH] va.h: Improve the comment on the encode framerate parameter

[Xiang, Haihao]

Applied, Thanks.

>-----Original Message-----
>From: Mark Thompson [mailto:sw at jkqxz.net]
>Sent: Friday, December 23, 2016 2:22 AM
>To: Xiang, Haihao <haihao.xiang at intel.com>; libva at lists.freedesktop.org
>Subject: [PATCH] va.h: Improve the comment on the encode framerate
>parameter
>
>Signed-off-by: Mark Thompson <sw at jkqxz.net>
>---
>On 22/12/16 01:32, Xiang, Haihao wrote:
>>
>>> Hmm.  Reading the comment in va/va.h again:
>>>
>>>     /*
>>>      * fps = numerator / denominator
>>>      * The high 2 bytes (bits 16 to 31) of framerate specifies the
>>> numerator, and
>>>      * the low 2 bytes (bits 0 to 15) of framerate specifies the
>>> denominator. For
>>>      * example, ((100 < 16 ) | 750) is 7.5 fps
>>>      *
>>>      * If the high 2 btyes is 0, the frame rate is specified by the
>>> low 2 bytes.
>>>      */
>>>     unsigned int framerate;
>>>
>>> the example and the text do not agree (I was following the text and
>>> didn't read the example carefully).  Looking at the previous code
>>> there, apparently the example is the one which should be followed?
>>
>> It was my fault when I added the comment. Yes, we should follow the
>> example, some applications have already uses the form of the example.
>> BTW the example should be ((100 << 16) | 750) :(
>>
>>> If you could confirm that this is the intention and will not break
>>> any other drivers, I will send a new patch to libva to fix the text
>>> of the comment (and also change my code to match).
>>
>> Yes, please.
>>
>
>Enclosing patch fixes the incorrect comment, and also adds a bit more
>explanation to hopefully make the use clearer.
>
>Thanks,
>
>- Mark
>
>
> va/va.h | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
>diff --git a/va/va.h b/va/va.h
>index 8791906..a1ab4a8 100644
>--- a/va/va.h
>+++ b/va/va.h
>@@ -1277,12 +1277,22 @@ typedef struct _VAEncMiscParameterRateControl
>typedef struct _VAEncMiscParameterFrameRate  {
>     /*
>-     * fps = numerator / denominator
>-     * The high 2 bytes (bits 16 to 31) of framerate specifies the numerator, and
>-     * the low 2 bytes (bits 0 to 15) of framerate specifies the denominator. For
>-     * example, ((100 < 16 ) | 750) is 7.5 fps
>+     * The framerate is specified as a number of frames per second, as a
>+     * fraction.  The denominator of the fraction is given in the top half
>+     * (the high two bytes) of the framerate field, and the numerator is
>+     * given in the bottom half (the low two bytes).
>      *
>-     * If the high 2 btyes is 0, the frame rate is specified by the low 2 bytes.
>+     * That is:
>+     * denominator = framerate >> 16 & 0xffff;
>+     * numerator   = framerate & 0xffff;
>+     * fps         = numerator / denominator;
>+     *
>+     * For example, if framerate is set to (100 << 16 | 750), this is
>+     * 750 / 100, hence 7.5fps.
>+     *
>+     * If the denominator is zero (the high two bytes are both zero) then
>+     * it takes the value one instead, so the framerate is just the integer
>+     * in the low 2 bytes.
>      */
>     unsigned int framerate;
>     union
>--
>2.11.0