Re: [libav-devel] [PATCH 1/4] vaapi_encode: Pass framerate parameters to driver

2017-01-29 Thread Mark Thompson 
On 24/01/17 08:20, Anton Khirnov wrote:
> Quoting Mark Thompson (2017-01-21 23:10:54)
>> On 13/01/17 11:07, Anton Khirnov wrote:
>>> Quoting Mark Thompson (2017-01-10 00:47:55)
 ---
 The i965 driver has been updated so that this works consistently.

  libavcodec/vaapi_encode.c | 14 ++
  libavcodec/vaapi_encode.h |  4 
  2 files changed, 18 insertions(+)

>>>
>>> Code itself looks ok, but won't old versions of the driver misbehave?
>>
>> It will make the bitrate-targetted RC modes for VP8 and VP9 do nasty things 
>> with the Intel driver before 1.8.0.
>>
>> Would you prefer this to have some sort of hack to try to parse the
>> Intel driver version out of the vendor string and fail out for some
>> codecs?  (That's the only way I can see to test it - the API is clear
>> that this has always been valid, it was just the driver which didn't
>> implement it properly.)
>>
>> For VP8 it's essentially irrelevant because the driver is so terrible
>> in current versions that noone in their right mind would ever use it
>> (there are now patches outstanding on the libva list which make it
>> usable).  For VP9 it's unfortunate because it does already work mostly
>> sensibly, but given how recent the hardware is one would hope that
>> people aren't going to blindly expect an earlier version of libva to
>> work properly?
> 
> I guess if it's just VP8 and VP9 then we shouldn't be too concerned too
> much. Perhaps just mention on the wiki or somewhere that old intel
> drivers don't work properly.

Urgh, it seems that mesa also has issues with this in the current version: 
.

Given that it's required for sensible support of VP8/VP9 (which don't carry any 
framerate in the bitstream and therefore it is not present in the sequence 
parameters here), I think making it conditional on VAAPI 0.40.0 is the least 
broken option.  That is the most recent bump and is not yet released, but the 
releases are synchronised such that correct support in the Intel driver will be 
available at the same time.  Older versions should then just not be given the 
framerate structure at all, because it can only confuse them.

- Mark
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/4] vaapi_encode: Pass framerate parameters to driver

2017-01-24 Thread Anton Khirnov 
Quoting Mark Thompson (2017-01-21 23:10:54)
> On 13/01/17 11:07, Anton Khirnov wrote:
> > Quoting Mark Thompson (2017-01-10 00:47:55)
> >> ---
> >> The i965 driver has been updated so that this works consistently.
> >>
> >>  libavcodec/vaapi_encode.c | 14 ++
> >>  libavcodec/vaapi_encode.h |  4 
> >>  2 files changed, 18 insertions(+)
> >>
> > 
> > Code itself looks ok, but won't old versions of the driver misbehave?
> 
> It will make the bitrate-targetted RC modes for VP8 and VP9 do nasty things 
> with the Intel driver before 1.8.0.
> 
> Would you prefer this to have some sort of hack to try to parse the
> Intel driver version out of the vendor string and fail out for some
> codecs?  (That's the only way I can see to test it - the API is clear
> that this has always been valid, it was just the driver which didn't
> implement it properly.)
> 
> For VP8 it's essentially irrelevant because the driver is so terrible
> in current versions that noone in their right mind would ever use it
> (there are now patches outstanding on the libva list which make it
> usable).  For VP9 it's unfortunate because it does already work mostly
> sensibly, but given how recent the hardware is one would hope that
> people aren't going to blindly expect an earlier version of libva to
> work properly?

I guess if it's just VP8 and VP9 then we shouldn't be too concerned too
much. Perhaps just mention on the wiki or somewhere that old intel
drivers don't work properly.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/4] vaapi_encode: Pass framerate parameters to driver

2017-01-21 Thread Mark Thompson 
On 13/01/17 11:07, Anton Khirnov wrote:
> Quoting Mark Thompson (2017-01-10 00:47:55)
>> ---
>> The i965 driver has been updated so that this works consistently.
>>
>>  libavcodec/vaapi_encode.c | 14 ++
>>  libavcodec/vaapi_encode.h |  4 
>>  2 files changed, 18 insertions(+)
>>
> 
> Code itself looks ok, but won't old versions of the driver misbehave?

It will make the bitrate-targetted RC modes for VP8 and VP9 do nasty things 
with the Intel driver before 1.8.0.

Would you prefer this to have some sort of hack to try to parse the Intel 
driver version out of the vendor string and fail out for some codecs?  (That's 
the only way I can see to test it - the API is clear that this has always been 
valid, it was just the driver which didn't implement it properly.)

For VP8 it's essentially irrelevant because the driver is so terrible in 
current versions that noone in their right mind would ever use it (there are 
now patches outstanding on the libva list which make it usable).  For VP9 it's 
unfortunate because it does already work mostly sensibly, but given how recent 
the hardware is one would hope that people aren't going to blindly expect an 
earlier version of libva to work properly?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/4] vaapi_encode: Pass framerate parameters to driver

2017-01-13 Thread Anton Khirnov 
Quoting Mark Thompson (2017-01-10 00:47:55)
> ---
> The i965 driver has been updated so that this works consistently.
> 
>  libavcodec/vaapi_encode.c | 14 ++
>  libavcodec/vaapi_encode.h |  4 
>  2 files changed, 18 insertions(+)
> 

Code itself looks ok, but won't old versions of the driver misbehave?

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 1/4] vaapi_encode: Pass framerate parameters to driver

2017-01-09 Thread Mark Thompson 
---
The i965 driver has been updated so that this works consistently.

 libavcodec/vaapi_encode.c | 14 ++
 libavcodec/vaapi_encode.h |  4 
 2 files changed, 18 insertions(+)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 246b76abc..953a6bae0 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1090,6 +1090,7 @@ static av_cold int 
vaapi_encode_init_rate_control(AVCodecContext *avctx)
 VAAPIEncodeContext *ctx = avctx->priv_data;
 int hrd_buffer_size;
 int hrd_initial_buffer_fullness;
+int num, den;
 
 if (avctx->rc_buffer_size)
 hrd_buffer_size = avctx->rc_buffer_size;
@@ -1124,6 +1125,19 @@ static av_cold int 
vaapi_encode_init_rate_control(AVCodecContext *avctx)
 ctx->global_params_size[ctx->nb_global_params++] =
 sizeof(ctx->hrd_params);
 
+if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
+av_reduce(, , avctx->framerate.num, avctx->framerate.den, 
65535);
+else
+av_reduce(, , avctx->time_base.den, avctx->time_base.num, 
65535);
+
+ctx->fr_params.misc.type = VAEncMiscParameterTypeFrameRate;
+ctx->fr_params.fr.framerate = (unsigned int)den << 16 | num;
+
+ctx->global_params[ctx->nb_global_params] =
+>fr_params.misc;
+ctx->global_params_size[ctx->nb_global_params++] =
+sizeof(ctx->fr_params);
+
 return 0;
 }
 
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 2a72510b8..e7f36eaf5 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -154,6 +154,10 @@ typedef struct VAAPIEncodeContext {
 VAEncMiscParameterBuffer misc;
 VAEncMiscParameterHRD hrd;
 } hrd_params;
+struct {
+VAEncMiscParameterBuffer misc;
+VAEncMiscParameterFrameRate fr;
+} fr_params;
 
 // Per-sequence parameter structure (VAEncSequenceParameterBuffer*).
 void   *codec_sequence_params;
-- 
2.11.0

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel