[ffmpeg-qsv] add string api support

[FocusLuo]

add string api support for ffmpeg-qsv
1, enable on DG2 fistly
2, the patch comments will update later after some times reviews done

[FocusLuo]

ffmpeg-qsv cmdline:
./smoke run test/ffmpeg-qsv/encode --pl DG2 --context=sanity --device=/dev/dri/renderD129 -vv
./smoke run test/ffmpeg-qsv/encode --pl DG2 --context=sanity --device=/dev/dri/renderD129 --stringapi 0 -vv
./smoke run test/ffmpeg-qsv/encode --pl DG2 --context=sanity --device=/dev/dri/renderD129 --stringapi 1 -vv
./smoke run test/ffmpeg-qsv/encode --pl DG2 --context=sanity --device=/dev/dri/renderD129 --stringapi 2 -vv
stringapi = 0 and others, will use traditional ffmpeg encode cmdline option
stringapi = 1, will use string api ffmpeg encode cmdline option
stringapi = 2, random to use traditional or string api ffmpeg encode cmdline option

[Bin-CI]

@mengker33 please help to review

[uartie]

This is not a full review...

I'm not convinced we should modify the base qsv encoder. It seems we could introduce a new qsv string encoder class or something. Let me think about the design some more and we can discuss later...

[FocusLuo]

This is not a full review...

I'm not convinced we should modify the base qsv encoder. It seems we could introduce a new qsv string encoder class or something. Let me think about the design some more and we can discuss later...

Cool! thanks a lot Artie on supporting this. BTW, gst-vpl will also support stringAPI in Q3, so please also think about gst-vpl part.

[uartie]

This is not a full review...
I'm not convinced we should modify the base qsv encoder. It seems we could introduce a new qsv string encoder class or something. Let me think about the design some more and we can discuss later...

Cool! thanks a lot Artie on supporting this. BTW, gst-vpl will also support stringAPI in Q3, so please also think about gst-vpl part.

@FocusLuo please have a look at uartie@714ea06
It uses multi-inheritance techniques to "mixin" the string api encoder overrides. It isn't a complete solution, yet, so I figure you may be able to finish some of the details (e.g. additional property overrides, mappings, etc.).
Note, at the end of the file is a quick example of usage. We may need to think about if we want to reuse existing user config spec variants or add a new variant. If we reuse, then we will essentially double the encode cases, which may not be desirable.

[FocusLuo]

This is not a full review...
I'm not convinced we should modify the base qsv encoder. It seems we could introduce a new qsv string encoder class or something. Let me think about the design some more and we can discuss later...

Cool! thanks a lot Artie on supporting this. BTW, gst-vpl will also support stringAPI in Q3, so please also think about gst-vpl part.

@FocusLuo please have a look at uartie@714ea06 It uses multi-inheritance techniques to "mixin" the string api encoder overrides. It isn't a complete solution, yet, so I figure you may be able to finish some of the details (e.g. additional property overrides, mappings, etc.). Note, at the end of the file is a quick example of usage. We may need to think about if we want to reuse existing user config spec variants or add a new variant. If we reuse, then we will essentially double the encode cases, which may not be desirable.

Thanks Artie, by re-used the uartie@714ea06,
re-submitted the patches, how do you think?

1. only enabled the avc/hevc/av1 encode firstly
2. there will be some failed cases on cbr and vbr, especailly for hevc and av1 encode, how to control the CI regression test report? do you have any idea?

[FocusLuo]

New test cases cmdlines: for avc/hevc/av1 encode
./smoke run test/ffmpeg-qsv/encode/hevc.py:cqp.test_strapi --pl ADL --context=sanity --device=/dev/dri/renderD128 -vv
./smoke run test/ffmpeg-qsv/encode/hevc.py:cqp_lp.test_strapi --pl DG2--context=sanity --device=/dev/dri/renderD129 -vv
./smoke run test/ffmpeg-qsv/encode/hevc.py:cbr.test_strapi --pl ADL --context=sanity --device=/dev/dri/renderD128 -vv
./smoke run test/ffmpeg-qsv/encode/hevc.py:cbr_lp.test_strapi --pl DG2--context=sanity --device=/dev/dri/renderD129 -vv
./smoke run test/ffmpeg-qsv/encode/hevc.py:vbr.test_strapi --pl ADL --context=sanity --device=/dev/dri/renderD128 -vv
./smoke run test/ffmpeg-qsv/encode/hevc.py:vbr_lp.test_strapi --pl DG2--context=sanity --device=/dev/dri/renderD129 -vv

[uartie]

This is not a full review...
I'm not convinced we should modify the base qsv encoder. It seems we could introduce a new qsv string encoder class or something. Let me think about the design some more and we can discuss later...

Cool! thanks a lot Artie on supporting this. BTW, gst-vpl will also support stringAPI in Q3, so please also think about gst-vpl part.

@FocusLuo please have a look at uartie@714ea06 It uses multi-inheritance techniques to "mixin" the string api encoder overrides. It isn't a complete solution, yet, so I figure you may be able to finish some of the details (e.g. additional property overrides, mappings, etc.). Note, at the end of the file is a quick example of usage. We may need to think about if we want to reuse existing user config spec variants or add a new variant. If we reuse, then we will essentially double the encode cases, which may not be desirable.

Thanks Artie, by re-used the uartie@714ea06, re-submitted the patches, how do you think?

1. only enabled the avc/hevc/av1 encode firstly
2. there will be some failed cases on cbr and vbr, especailly for hevc and av1 encode, how to control the CI regression test report? do you have any idea?

My patch wasn't a completed solution... please finish the TODO's and other details (missing some property definitions and mappings).

[FocusLuo]

@uartie do you think it is good to merge this PR firstly? then let me to submit another PR for your other comments such as

1. move map_level/rate/profile to the enum class as lookup;
2. to use new gen functions for string (there will have avc gen cqp/cbr/vbr, hevc gen functions, av1 gen functions and also have lp and non-lp),
   and definitely you will have other comments input in future.
   how do you think? this will help me decrease my local update/comment maintain effort, and also help wenqing to do some issue testing on finding issue among different platforms or integrate into CI issue firstly
   thanks
   Focus

[uartie]

@uartie do you think it is good to merge this PR firstly? then let me to submit another PR for your other comments such as

1. move map_level/rate/profile to the enum class as lookup;
2. to use new gen functions for string (there will have avc gen cqp/cbr/vbr, hevc gen functions, av1 gen functions and also have lp and non-lp),
   and definitely you will have other comments input in future.
   how do you think? this will help me decrease my local update/comment maintain effort, and also help wenqing to do some issue testing on finding issue among different platforms or integrate into CI issue firstly
   thanks
   Focus

The issue I have is it automatically doubles the cases as mentioned in #651 (comment). For full tests, it would add an additional 2-3 hours depending on platform.

[FocusLuo]

@uartie do you think it is good to merge this PR firstly? then let me to submit another PR for your other comments such as

1. move map_level/rate/profile to the enum class as lookup;
2. to use new gen functions for string (there will have avc gen cqp/cbr/vbr, hevc gen functions, av1 gen functions and also have lp and non-lp),
   and definitely you will have other comments input in future.
   how do you think? this will help me decrease my local update/comment maintain effort, and also help wenqing to do some issue testing on finding issue among different platforms or integrate into CI issue firstly
   thanks
   Focus

The issue I have is it automatically doubles the cases as mentioned in #651 (comment). For full tests, it would add an additional 2-3 hours depending on platform.

Ok, will submit new patch to use dedicated gen functions for string api, thanks

[Bin-CI]

Almost good to me, just one comment

[Bin-CI]

LGTM, look's like we can merge have_ffmpeg_filter with have_ffmpeg_encoder, have_ffmpeg_decoder and have_ffmpeg_filter_options have_ffmpeg_encode_options in feature

def _have_ffmpeg_key(name, key):
  result = try_call(f"{exe2os('ffmpeg')} -hide_banner -{key}s | awk '{{print $2}}' | grep -w {name}")
  return result, name

[FocusLuo]

@uartie is the latest new update version good to you?

[uartie]

Don't use is True.

The variants should always be a list before hitting the loop. This will cut down on the diff noise.

[uartie]

Please avoid making extra changes that weren't suggested and we could be good to go. Almost there ;)

[uartie]

The end result looks good to me.

However, please squash your commits into a few logically organized commits, as appropriate. There should not be any intermediate commits that only address the reviews. Those changes should be applied to the originals.

[FocusLuo]

The end result looks good to me.

However, please squash your commits into a few logically organized commits, as appropriate. There should not be any intermediate commits that only address the reviews. Those changes should be applied to the originals.

thanks Artie,
updated

[uartie]

Ugh... sorry I didn't notice these before...