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

[Mark Thompson]

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