Skip to content

oapve param parse #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kpchoi wants to merge 7 commits into from
Closed

oapve param parse #55

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6145e6f
modification of decoding tile
mss-park Mar 7, 2025
6858c97
refactoring
kpchoi Mar 7, 2025
48f696f
refactoring
kpchoi Mar 7, 2025
4772dca
added xeve_param_parse() API
kpchoi Mar 7, 2025
3aaf2ff
fix for arm buiding
kpchoi Mar 7, 2025
4761131
fix typo
kpchoi Mar 7, 2025
c17a1d0
added 'preset' to oapve_param_parse()
kpchoi Mar 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions inc/oapv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ oapve_t OAPV_EXPORT oapve_create(oapve_cdesc_t *cdesc, int *err);
void OAPV_EXPORT oapve_delete(oapve_t eid);
int OAPV_EXPORT oapve_config(oapve_t eid, int cfg, void *buf, int *size);
int OAPV_EXPORT oapve_param_default(oapve_param_t *param);
int OAPV_EXPORT oapve_param_parse(oapve_param_t* param, const char* name, const char* value);
int OAPV_EXPORT oapve_encode(oapve_t eid, oapv_frms_t *ifrms, oapvm_t mid, oapv_bitb_t *bitb, oapve_stat_t *stat, oapv_frms_t *rfrms);

/*****************************************************************************
Expand Down
136 changes: 134 additions & 2 deletions src/oapv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1486,6 +1486,128 @@ int oapve_param_default(oapve_param_t *param)

return OAPV_OK;
}
///////////////////////////////////////////////////////////////////////////////
// parameter parsing helper function for encoder

#include <stddef.h> // for ''offsetof()'

// clang-format off
#define PARAMS_END_KEY (0)
#define PARAM_STR_MAX_LEN (256)

#define DT_INTEGER (1 << 0) /* integer type value */
#define DT_DOUBLE (1 << 1) /* double type value */
#define DT_STRING (1 << 2) /* string type value */

#define OFFSET(x) offsetof(oapve_param_t, x)

#define ENC_SET_PARAM_DATA(param, dtype) \
{ .name=#param, .type=dtype, .offset=OFFSET(param) }

typedef struct enc_param_data {
const char *name; /* text string conneced to param of a given name */
int type; /* data type for a given param */
int offset; /* the offset relative to the oapve_param_t structure where the param value is stored */
} enc_param_data_t;

/* define various command line options as a table */
static const enc_param_data_t enc_params[] = {
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 ),
Comment on lines 1515 to 1539
Copy link
Collaborator

@dariusz-f dariusz-f Mar 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are here is all required options are listed?
For example threads is missing.

Copy link
Collaborator Author

@kpchoi kpchoi Mar 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to add more variables.
Could you help to add "unsigned char q_matrix[OAPV_MAX_CC][OAPV_BLK_D];" option?

Copy link
Collaborator Author

@kpchoi kpchoi Mar 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

@cpncf cpncf Mar 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@dkozinski dkozinski Mar 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@kpchoi kpchoi Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will refactor many code for supporting oapve_param_parse() API.
And then I will commit PR again later.


/* termination */
{ .name = PARAMS_END_KEY }
};
// clang-format on

static int enc_param_search_name(const char * name)
{
int idx = 0;
const enc_param_data_t* p = enc_params;

while(p->name != PARAMS_END_KEY)
{
if(!strcmp(name, p->name))
{
return idx;
}
idx++;
p++;
}
return -1;
}

int oapve_param_parse(oapve_param_t *param, const char *name, const char *value)
{
int ival;
double dval;
char *endptr;
const enc_param_data_t* p = enc_params;

if(!param || !name || !value) {
return OAPV_ERR_INVALID_ARGUMENT;
}

int idx = enc_param_search_name(name);
if( idx < 0 )
return OAPV_ERR_INVALID_ARGUMENT;

p = enc_params + idx;

switch(p->type) {
case DT_INTEGER:
ival = strtol(value, &endptr, 10);
if (*endptr != '\0')
return OAPV_ERR_INVALID_ARGUMENT;

*((int*)((char*)param + p->offset)) = ival;

break;
case DT_DOUBLE:
dval = strtod(value, &endptr);
if (*endptr != '\0')
return OAPV_ERR_INVALID_ARGUMENT;

*((double*)((char*)param + p->offset)) = dval;

break;
case DT_STRING:

strncpy((char*)((char*)param + p->offset), value, PARAM_STR_MAX_LEN);

// If PARAM_STR_MAX_LEN is less than or equal to the length of val,
// a null character (\0) is not appended to the copied string (char*)(args->opts[idx].opt_storage)
// The line below prevents truncation of destination string to not-null terminated string
((char*)((char*)param + p->offset))[PARAM_STR_MAX_LEN-1] = 0;

break;
default:
return OAPV_ERR;
}
return OAPV_OK;
}

///////////////////////////////////////////////////////////////////////////////
// enc of encoder code
Expand Down Expand Up @@ -1670,18 +1792,22 @@ static int dec_tile_comp(oapvd_tile_t *tile, oapvd_ctx_t *ctx, oapvd_core_t *cor

/* byte align */
oapv_bsr_align8(bs);
/* check actual read size of 'tile()' is equal or smaller than 'tile_data_size' in tile header */
oapv_assert_rv(BSR_GET_READ_BYTE(bs) <= tile->th.tile_data_size[c], OAPV_ERR_MALFORMED_BITSTREAM);

return OAPV_OK;
}

static int dec_tile(oapvd_core_t *core, oapvd_tile_t *tile)
{
int ret, midx, x, y, c;
oapvd_ctx_t *ctx = core->ctx;
oapv_bs_t bs;
oapv_bs_t bs; // bs for 'tile()' syntax

oapv_bsr_init(&bs, tile->bs_beg + OAPV_TILE_SIZE_LEN, tile->data_size, NULL);
ret = oapvd_vlc_tile_header(&bs, ctx, &tile->th);
oapv_assert_rv(OAPV_SUCCEEDED(ret), ret);

for(c = 0; c < ctx->num_comp; c++) {
core->qp[c] = tile->th.tile_qp[c];
int dq_scale = oapv_tbl_dq_scale[core->qp[c] % 6];
Expand All @@ -1702,6 +1828,9 @@ static int dec_tile(oapvd_core_t *core, oapvd_tile_t *tile)
for(c = 0; c < ctx->num_comp; c++) {
int tc, s_dst;
s16 *dst;
oapv_bs_t bsc; // bs for 'tile_data()' syntax

oapv_bsr_init(&bsc, BSR_GET_CUR(&bs), tile->th.tile_data_size[c], NULL);

if(OAPV_CS_GET_FORMAT(ctx->imgb->cs) == OAPV_CF_PLANAR2) {
tc = c > 0 ? 1 : 0;
Expand All @@ -1714,8 +1843,11 @@ static int dec_tile(oapvd_core_t *core, oapvd_tile_t *tile)
s_dst = ctx->imgb->s[c];
}

ret = dec_tile_comp(tile, ctx, core, &bs, c, s_dst, dst);
ret = dec_tile_comp(tile, ctx, core, &bsc, c, s_dst, dst);
oapv_assert_rv(OAPV_SUCCEEDED(ret), ret);

// move bs buffer to next 'tile_data()' component
BSR_MOVE_BYTE_ALIGN(&bs, tile->th.tile_data_size[c]);
}

oapvd_vlc_tile_dummy_data(&bs);
Expand Down