Conversation
Signed-off-by: kp5.choi@samsung.com <kp5.choi@samsung.com>
Signed-off-by: kp5.choi@samsung.com <kp5.choi@samsung.com>
Signed-off-by: kp5.choi@samsung.com <kp5.choi@samsung.com>
Signed-off-by: kp5.choi@samsung.com <kp5.choi@samsung.com>
|
Please change the name in the title of the pull request and in the commit message from "xeve" to "oapv." |
| ENC_SET_PARAM_DATA(profile_idc, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(level_idc, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(band_idc, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(w, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(h, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(fps_num, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(fps_den, DT_INTEGER), | ||
| ENC_SET_PARAM_DATA(rc_type, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(qp, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(qp_offset_c1, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(qp_offset_c2, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(qp_offset_c3, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(bitrate, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(use_filler, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(csp, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(tile_cols, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(tile_rows, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(tile_w_mb, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(tile_h_mb, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(color_description_present_flag, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(color_primaries, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(transfer_characteristics, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(matrix_coefficients, DT_INTEGER ), | ||
| ENC_SET_PARAM_DATA(full_range_flag, DT_INTEGER ), |
There was a problem hiding this comment.
Are here is all required options are listed?
For example threads is missing.
There was a problem hiding this comment.
I've tried to add more variables.
Could you help to add "unsigned char q_matrix[OAPV_MAX_CC][OAPV_BLK_D];" option?
There was a problem hiding this comment.
And, some options are not listed in the oapve_param_t. like 'threads'. it is another issue.
Shall we move 'threads' variable to 'oapve_param_t' structure?
There was a problem hiding this comment.
How about not supporting '-oapv-params' options?
Instead of it, we can add more AVOptions to support every options listed in oapve_params_t.
Before create new instance by calling oapve_create(), the member variables of oapve_params_t can be directly changed without any side-effect.
There was a problem hiding this comment.
Theoretically, we could eliminate the '-oapv-params' option and map all codec parameters (fields of the oapve_param structure) to options (AVOption in the implementation of FFCodec ff_libapv_encoder) in FFmpeg. However, I am not sure if this is a good solution.
Let's assume that we map all OpenAPV codec parameters in ffapv to AVOptions.
In that case, if we want to provide access to all OAPV codec parameters from the FFmpeg side, it will require the maintainers of ffapv to continuously check for any changes in the OpenAPV library and synchronize those changes with FFmpeg.
After implementing changes in OAPV that require modifications on the FFmpeg side, we will have to wait for the corresponding changes to be made in FFmpeg, then go through the review process, be accepted by the FFmpeg community, and merged into the master branch of the FFmpeg project. As you can see, synchronization will involve a certain time overhead, which we will have little influence over due to the engagement of the FFmpeg community.
I think that the optimal solution is to map the most commonly used codec parameters to options in FFmpeg (as is currently the case) and allow the setting of any parameter through the OpenAPV library's API.
This means that the OpenAPV library should expose an API that allows setting all possible codec parameters (function int OAPV_EXPORT oapve_param_parse(oapve_param_t* param, const char* name, const char* value);).
Exposing the oapve_param_parse function in the OpenAPV library allows for setting all supported codec parameters using the -oapv-params option on the FFmpeg side.
This way, no changes in the OpenAPV library related to codec parameters will require reimplementation on the ffapv side.
Moreover, the FFmpeg user, by using the -oapv-params option, will be able to directly refer to the liboapv documentation when configuring codec parameters.
There was a problem hiding this comment.
I will refactor many code for supporting oapve_param_parse() API.
And then I will commit PR again later.
Signed-off-by: kp5.choi@samsung.com <kp5.choi@samsung.com>
5 participants
Uh oh!
There was an error while loading. Please reload this page.