diff mbox

[FFmpeg-devel] libavcodec/qsvenc_hevc: correction for QSV HEVC default plugin selection on Windows

Message ID a8e74b62-55b9-ac87-b366-62f2f530cd4e@landgraph.ru
State New
Headers show

Commit Message

Landgraph Oct. 6, 2018, 6:21 a.m. UTC 
1. Old logic meaned: everywhere, except Windows, ffmpeg has to use HW 
acceleration, but on Windows ffmpeg has to use (unavailable) software 
encode by default
2. Software encode is available only if you provide corresponding 
software MediaSDK library, which isn't provided with ffmpeg. More 
information could be found in 
https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/readme-encode_linux.pdf
3. HW encode is available on Windows in the driver by default
---
  libavcodec/qsvenc_hevc.c | 4 ----
  1 file changed, 4 deletions(-)

Comments

Max Dmitrichenko Oct. 12, 2018, 10:27 a.m. UTC | #1 
On Tue, Oct 9, 2018 at 3:50 AM Landgraph <me@landgraph.ru> wrote:

> 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use HW
> acceleration, but on Windows ffmpeg has to use (unavailable) software
> encode by default
> 2. Software encode is available only if you provide corresponding
> software MediaSDK library, which isn't provided with ffmpeg. More
> information could be found in
>
> https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/readme-encode_linux.pdf
> 3. HW encode is available on Windows in the driver by default
> ---
>   libavcodec/qsvenc_hevc.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
> index 4339b316a3..e7ca27d49f 100644
> --- a/libavcodec/qsvenc_hevc.c
> +++ b/libavcodec/qsvenc_hevc.c
> @@ -217,11 +217,7 @@ static av_cold int qsv_enc_close(AVCodecContext
> *avctx)
>       return ff_qsv_enc_close(avctx, &q->qsv);
>   }
>
> -#if defined(_WIN32)
> -#define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_SW
> -#else
>   #define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_HW
> -#endif
>
>   #define OFFSET(x) offsetof(QSVHEVCEncContext, x)
>   #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> --
> 2.14.1.windows.1
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


it is good switch for HW configuration per default, agree.
LGTM

regards
Maxym
Mark Thompson Oct. 13, 2018, 4:37 p.m. UTC | #2 
On 06/10/18 07:21, Landgraph wrote:
> 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use HW acceleration, but on Windows ffmpeg has to use (unavailable) software encode by default
> 2. Software encode is available only if you provide corresponding software MediaSDK library, which isn't provided with ffmpeg. More information could be found in https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/readme-encode_linux.pdf
> 3. HW encode is available on Windows in the driver by default

This has been proposed before - I can't find the original discussion (maybe it was on IRC), but I did find <https://lists.libav.org/pipermail/libav-devel/2016-November/080419.html>.  The reason for not doing it is that a subset of the Intel drivers segfault immediately when the hardware plugin is loaded on some platforms.  That's a pain for anyone wanting to support diverse systems, so the decision was to continue to load the sw plugin by default so it doesn't crash (even if the software plugin isn't present), and leave the non-default case as the crashing one so the user has to do something to trigger it.

If you can characterise either the set of platforms it crashes on or a set of platforms it definitely works on then maybe we could conditionally change the default behaviour?

- Mark


> ---
>  libavcodec/qsvenc_hevc.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
> index 4339b316a3..e7ca27d49f 100644
> --- a/libavcodec/qsvenc_hevc.c
> +++ b/libavcodec/qsvenc_hevc.c
> @@ -217,11 +217,7 @@ static av_cold int qsv_enc_close(AVCodecContext *avctx)
>      return ff_qsv_enc_close(avctx, &q->qsv);
>  }
> 
> -#if defined(_WIN32)
> -#define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_SW
> -#else
>  #define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_HW
> -#endif
> 
>  #define OFFSET(x) offsetof(QSVHEVCEncContext, x)
>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
Max Dmitrichenko Oct. 13, 2018, 7:35 p.m. UTC | #3 
On Sat, Oct 13, 2018 at 6:43 PM Mark Thompson <sw@jkqxz.net> wrote:

> On 06/10/18 07:21, Landgraph wrote:
> > 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use HW
> acceleration, but on Windows ffmpeg has to use (unavailable) software
> encode by default
> > 2. Software encode is available only if you provide corresponding
> software MediaSDK library, which isn't provided with ffmpeg. More
> information could be found in
> https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/readme-encode_linux.pdf
> > 3. HW encode is available on Windows in the driver by default
>
> This has been proposed before - I can't find the original discussion
> (maybe it was on IRC), but I did find <
> https://lists.libav.org/pipermail/libav-devel/2016-November/080419.html>.
> The reason for not doing it is that a subset of the Intel drivers segfault
> immediately when the hardware plugin is loaded on some platforms.  That's a
> pain for anyone wanting to support diverse systems, so the decision was to
> continue to load the sw plugin by default so it doesn't crash (even if the
> software plugin isn't present), and leave the non-default case as the
> crashing one so the user has to do something to trigger it.
>
> If you can characterise either the set of platforms it crashes on or a set
> of platforms it definitely works on then maybe we could conditionally
> change the default behaviour?
>
> - Mark
>
>
it was 2 years old discussion and with early drivers (we even had this
development a bit ahead of general driver availability)
now it should be working on most of the platforms - I would suggest to make
a positive side.



>
> > ---
> >  libavcodec/qsvenc_hevc.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
> > index 4339b316a3..e7ca27d49f 100644
> > --- a/libavcodec/qsvenc_hevc.c
> > +++ b/libavcodec/qsvenc_hevc.c
> > @@ -217,11 +217,7 @@ static av_cold int qsv_enc_close(AVCodecContext
> *avctx)
> >      return ff_qsv_enc_close(avctx, &q->qsv);
> >  }
> >
> > -#if defined(_WIN32)
> > -#define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_SW
> > -#else
> >  #define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_HW
> > -#endif
> >
> >  #define OFFSET(x) offsetof(QSVHEVCEncContext, x)
> >  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Zhong Li Oct. 23, 2018, 7:14 a.m. UTC | #4 
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Maxym Dmytrychenko

> Sent: Sunday, October 14, 2018 3:36 AM

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc: correction for

> QSV HEVC default plugin selection on Windows

> 

> On Sat, Oct 13, 2018 at 6:43 PM Mark Thompson <sw@jkqxz.net> wrote:

> 

> > On 06/10/18 07:21, Landgraph wrote:

> > > 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use

> > > HW

> > acceleration, but on Windows ffmpeg has to use (unavailable) software

> > encode by default

> > > 2. Software encode is available only if you provide corresponding

> > software MediaSDK library, which isn't provided with ffmpeg. More

> > information could be found in

> >

> https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/r

> e

> > adme-encode_linux.pdf

> > > 3. HW encode is available on Windows in the driver by default

> >

> > This has been proposed before - I can't find the original discussion

> > (maybe it was on IRC), but I did find <

> >

> https://lists.libav.org/pipermail/libav-devel/2016-November/080419.html>.

> > The reason for not doing it is that a subset of the Intel drivers

> > segfault immediately when the hardware plugin is loaded on some

> > platforms.  That's a pain for anyone wanting to support diverse

> > systems, so the decision was to continue to load the sw plugin by

> > default so it doesn't crash (even if the software plugin isn't

> > present), and leave the non-default case as the crashing one so the user

> has to do something to trigger it.

> >

> > If you can characterise either the set of platforms it crashes on or a

> > set of platforms it definitely works on then maybe we could

> > conditionally change the default behaviour?

> >

> > - Mark

> >

> >

> it was 2 years old discussion and with early drivers (we even had this

> development a bit ahead of general driver availability) now it should be

> working on most of the platforms - I would suggest to make a positive side.


Basically, HEVC HW encoding should be the default case if HW platform can support. 
If crashed with some specified drivers, thus should be a driver issue instead of hiding it in ffmpeg level. 
So, I agree with Maxym and the patch LGTM. 
(Of course, if we can verified on the platforms which was crashed as two years ago, 
that should be fine. However, IMHO this is not MUST. If it is still crash, reporting a bug to the driver developer should be the right way.)
Landgraph Dec. 5, 2018, 8:22 p.m. UTC | #5 
Hi All,

This is my first commit to ffmpeg, what should I do to merge it?

Do we have any reasons to not merge this?

Thanks!

23.10.2018 10:14, Li, Zhong пишет:
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
>> Of Maxym Dmytrychenko
>> Sent: Sunday, October 14, 2018 3:36 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc: correction for
>> QSV HEVC default plugin selection on Windows
>>
>> On Sat, Oct 13, 2018 at 6:43 PM Mark Thompson <sw@jkqxz.net> wrote:
>>
>>> On 06/10/18 07:21, Landgraph wrote:
>>>> 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use
>>>> HW
>>> acceleration, but on Windows ffmpeg has to use (unavailable) software
>>> encode by default
>>>> 2. Software encode is available only if you provide corresponding
>>> software MediaSDK library, which isn't provided with ffmpeg. More
>>> information could be found in
>>>
>> https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/r
>> e
>>> adme-encode_linux.pdf
>>>> 3. HW encode is available on Windows in the driver by default
>>> This has been proposed before - I can't find the original discussion
>>> (maybe it was on IRC), but I did find <
>>>
>> https://lists.libav.org/pipermail/libav-devel/2016-November/080419.html>.
>>> The reason for not doing it is that a subset of the Intel drivers
>>> segfault immediately when the hardware plugin is loaded on some
>>> platforms.  That's a pain for anyone wanting to support diverse
>>> systems, so the decision was to continue to load the sw plugin by
>>> default so it doesn't crash (even if the software plugin isn't
>>> present), and leave the non-default case as the crashing one so the user
>> has to do something to trigger it.
>>> If you can characterise either the set of platforms it crashes on or a
>>> set of platforms it definitely works on then maybe we could
>>> conditionally change the default behaviour?
>>>
>>> - Mark
>>>
>>>
>> it was 2 years old discussion and with early drivers (we even had this
>> development a bit ahead of general driver availability) now it should be
>> working on most of the platforms - I would suggest to make a positive side.
> Basically, HEVC HW encoding should be the default case if HW platform can support.
> If crashed with some specified drivers, thus should be a driver issue instead of hiding it in ffmpeg level.
> So, I agree with Maxym and the patch LGTM.
> (Of course, if we can verified on the platforms which was crashed as two years ago,
> that should be fine. However, IMHO this is not MUST. If it is still crash, reporting a bug to the driver developer should be the right way.)
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Zhong Li Dec. 6, 2018, 5:21 a.m. UTC | #6 
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Landgraph

> Sent: Thursday, December 6, 2018 4:23 AM

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc: correction for

> QSV HEVC default plugin selection on Windows

> 

> Hi All,

> 

> This is my first commit to ffmpeg, what should I do to merge it?

> 

> Do we have any reasons to not merge this?

> 

> Thanks!


Will apply if nobody against now. 

> 23.10.2018 10:14, Li, Zhong пишет:

> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> Behalf

> >> Of Maxym Dmytrychenko

> >> Sent: Sunday, October 14, 2018 3:36 AM

> >> To: ffmpeg-devel@ffmpeg.org

> >> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc:

> >> correction for QSV HEVC default plugin selection on Windows

> >>

> >> On Sat, Oct 13, 2018 at 6:43 PM Mark Thompson <sw@jkqxz.net> wrote:

> >>

> >>> On 06/10/18 07:21, Landgraph wrote:

> >>>> 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use

> >>>> HW

> >>> acceleration, but on Windows ffmpeg has to use (unavailable)

> >>> software encode by default

> >>>> 2. Software encode is available only if you provide corresponding

> >>> software MediaSDK library, which isn't provided with ffmpeg. More

> >>> information could be found in

> >>>

> >>

> https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/r

> >> e

> >>> adme-encode_linux.pdf

> >>>> 3. HW encode is available on Windows in the driver by default

> >>> This has been proposed before - I can't find the original discussion

> >>> (maybe it was on IRC), but I did find <

> >>>

> >>

> https://lists.libav.org/pipermail/libav-devel/2016-November/080419.html>.

> >>> The reason for not doing it is that a subset of the Intel drivers

> >>> segfault immediately when the hardware plugin is loaded on some

> >>> platforms.  That's a pain for anyone wanting to support diverse

> >>> systems, so the decision was to continue to load the sw plugin by

> >>> default so it doesn't crash (even if the software plugin isn't

> >>> present), and leave the non-default case as the crashing one so the

> >>> user

> >> has to do something to trigger it.

> >>> If you can characterise either the set of platforms it crashes on or

> >>> a set of platforms it definitely works on then maybe we could

> >>> conditionally change the default behaviour?

> >>>

> >>> - Mark

> >>>

> >>>

> >> it was 2 years old discussion and with early drivers (we even had

> >> this development a bit ahead of general driver availability) now it

> >> should be working on most of the platforms - I would suggest to make a

> positive side.

> > Basically, HEVC HW encoding should be the default case if HW platform

> can support.

> > If crashed with some specified drivers, thus should be a driver issue instead

> of hiding it in ffmpeg level.

> > So, I agree with Maxym and the patch LGTM.

> > (Of course, if we can verified on the platforms which was crashed as

> > two years ago, that should be fine. However, IMHO this is not MUST. If

> > it is still crash, reporting a bug to the driver developer should be

> > the right way.)

> >
Mark Thompson Dec. 9, 2018, 6:47 p.m. UTC | #7 
On 06/12/2018 05:21, Li, Zhong wrote:
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
>> Of Landgraph
>> Sent: Thursday, December 6, 2018 4:23 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc: correction for
>> QSV HEVC default plugin selection on Windows
>>
>> Hi All,
>>
>> This is my first commit to ffmpeg, what should I do to merge it?
>>
>> Do we have any reasons to not merge this?
>>
>> Thanks!
> 
> Will apply if nobody against now. 

Please add a note to the commit message explaining the original problem so that if anyone does come across it they can understand what's going on.  Old drivers do hang around for a long time, especially with the OEM-locked ones.

Ok with that change.

Thanks,

- Mark


>> 23.10.2018 10:14, Li, Zhong пишет:
>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On
>> Behalf
>>>> Of Maxym Dmytrychenko
>>>> Sent: Sunday, October 14, 2018 3:36 AM
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc:
>>>> correction for QSV HEVC default plugin selection on Windows
>>>>
>>>> On Sat, Oct 13, 2018 at 6:43 PM Mark Thompson <sw@jkqxz.net> wrote:
>>>>
>>>>> On 06/10/18 07:21, Landgraph wrote:
>>>>>> 1. Old logic meaned: everywhere, except Windows, ffmpeg has to use
>>>>>> HW
>>>>> acceleration, but on Windows ffmpeg has to use (unavailable)
>>>>> software encode by default
>>>>>> 2. Software encode is available only if you provide corresponding
>>>>> software MediaSDK library, which isn't provided with ffmpeg. More
>>>>> information could be found in
>>>>>
>>>>
>> https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/samples/r
>>>> e
>>>>> adme-encode_linux.pdf
>>>>>> 3. HW encode is available on Windows in the driver by default
>>>>> This has been proposed before - I can't find the original discussion
>>>>> (maybe it was on IRC), but I did find <
>>>>>
>>>>
>> https://lists.libav.org/pipermail/libav-devel/2016-November/080419.html>.
>>>>> The reason for not doing it is that a subset of the Intel drivers
>>>>> segfault immediately when the hardware plugin is loaded on some
>>>>> platforms.  That's a pain for anyone wanting to support diverse
>>>>> systems, so the decision was to continue to load the sw plugin by
>>>>> default so it doesn't crash (even if the software plugin isn't
>>>>> present), and leave the non-default case as the crashing one so the
>>>>> user
>>>> has to do something to trigger it.
>>>>> If you can characterise either the set of platforms it crashes on or
>>>>> a set of platforms it definitely works on then maybe we could
>>>>> conditionally change the default behaviour?
>>>>>
>>>>> - Mark
>>>>>
>>>>>
>>>> it was 2 years old discussion and with early drivers (we even had
>>>> this development a bit ahead of general driver availability) now it
>>>> should be working on most of the platforms - I would suggest to make a
>> positive side.
>>> Basically, HEVC HW encoding should be the default case if HW platform
>> can support.
>>> If crashed with some specified drivers, thus should be a driver issue instead
>> of hiding it in ffmpeg level.
>>> So, I agree with Maxym and the patch LGTM.
>>> (Of course, if we can verified on the platforms which was crashed as
>>> two years ago, that should be fine. However, IMHO this is not MUST. If
>>> it is still crash, reporting a bug to the driver developer should be
>>> the right way.)
>>>
Zhong Li Dec. 10, 2018, 5:08 a.m. UTC | #8 
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Mark Thompson

> Sent: Monday, December 10, 2018 2:48 AM

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc: correction for

> QSV HEVC default plugin selection on Windows

> 

> On 06/12/2018 05:21, Li, Zhong wrote:

> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> Behalf

> >> Of Landgraph

> >> Sent: Thursday, December 6, 2018 4:23 AM

> >> To: ffmpeg-devel@ffmpeg.org

> >> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc_hevc:

> >> correction for QSV HEVC default plugin selection on Windows

> >>

> >> Hi All,

> >>

> >> This is my first commit to ffmpeg, what should I do to merge it?

> >>

> >> Do we have any reasons to not merge this?

> >>

> >> Thanks!

> >

> > Will apply if nobody against now.

> 

> Please add a note to the commit message explaining the original problem so

> that if anyone does come across it they can understand what's going on.

> Old drivers do hang around for a long time, especially with the OEM-locked

> ones.


Sure, will add message to raise the risk of old windows driver when merge.

> Ok with that change.


Should be a good idea to bump a micro version for this change?
diff mbox

Patch

diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
index 4339b316a3..e7ca27d49f 100644
--- a/libavcodec/qsvenc_hevc.c
+++ b/libavcodec/qsvenc_hevc.c
@@ -217,11 +217,7 @@  static av_cold int qsv_enc_close(AVCodecContext *avctx)
      return ff_qsv_enc_close(avctx, &q->qsv);
  }

-#if defined(_WIN32)
-#define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_SW
-#else
  #define LOAD_PLUGIN_DEFAULT LOAD_PLUGIN_HEVC_HW
-#endif

  #define OFFSET(x) offsetof(QSVHEVCEncContext, x)
  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM