[FFmpeg-devel] [PATCH v2 1/3] avfilter: add v360 filter
Paul B Mahol
onemda at gmail.com
Wed Aug 14 17:09:27 EEST 2019
On Wed, Aug 14, 2019 at 4:06 PM Thilo Borgmann <thilo.borgmann at mail.de>
wrote:
> Am 14.08.19 um 15:14 schrieb Eugene:
> > On 14.08.2019 13:06, Paul B Mahol wrote:
> >
> >> On Wed, Aug 14, 2019 at 11:56 AM Thilo Borgmann <thilo.borgmann at mail.de
> >
> >> wrote:
> >>
> >>> [...]
> >>>
> >>>>>> +static int config_output(AVFilterLink *outlink)
> >>>>>> +{
> >>>>>> + AVFilterContext *ctx = outlink->src;
> >>>>>> + AVFilterLink *inlink = ctx->inputs[0];
> >>>>>> + V360Context *s = ctx->priv;
> >>>>>> + const AVPixFmtDescriptor *desc =
> >>>>>> av_pix_fmt_desc_get(inlink->format);
> >>>>>> + const int depth = desc->comp[0].depth;
> >>>>>> + float remap_data_size = 0.f;
> >>>>>> + int sizeof_remap;
> >>>>>> + int err;
> >>>>>> + int p, h, w;
> >>>>>> + float hf, wf;
> >>>>>> + float mirror_modifier[3];
> >>>>>> + void (*in_transform)(const V360Context *s,
> >>>>>> + const float *vec, int width, int height,
> >>>>>> + uint16_t us[4][4], uint16_t vs[4][4],
> float
> >>>>>> *du, float *dv);
> >>>>>> + void (*out_transform)(const V360Context *s,
> >>>>>> + int i, int j, int width, int height,
> >>>>>> + float *vec);
> >>>>>> + void (*calculate_kernel)(float du, float dv, int shift, const
> >>>>> XYRemap4
> >>>>>> *r_tmp, void *r);
> >>>>>> + float rot_mat[3][3];
> >>>>>> +
> >>>>>> + switch (s->interp) {
> >>>>>> + case NEAREST:
> >>>>>> + calculate_kernel = nearest_kernel;
> >>>>>> + s->remap_slice = depth <= 8 ? remap1_8bit_slice :
> >>>>>> remap1_16bit_slice;
> >>>>>> + sizeof_remap = sizeof(XYRemap1);
> >>>>>> + break;
> >>>>>> + case BILINEAR:
> >>>>>> + calculate_kernel = bilinear_kernel;
> >>>>>> + s->remap_slice = depth <= 8 ? remap2_8bit_slice :
> >>>>>> remap2_16bit_slice;
> >>>>>> + sizeof_remap = sizeof(XYRemap2);
> >>>>>> + break;
> >>>>>> + case BICUBIC:
> >>>>>> + calculate_kernel = bicubic_kernel;
> >>>>>> + s->remap_slice = depth <= 8 ? remap4_8bit_slice :
> >>>>>> remap4_16bit_slice;
> >>>>>> + sizeof_remap = sizeof(XYRemap4);
> >>>>>> + break;
> >>>>>> + case LANCZOS:
> >>>>>> + calculate_kernel = lanczos_kernel;
> >>>>>> + s->remap_slice = depth <= 8 ? remap4_8bit_slice :
> >>>>>> remap4_16bit_slice;
> >>>>>> + sizeof_remap = sizeof(XYRemap4);
> >>>>>> + break;
> >>>>>> + }
> >>>>>> +
> >>>>>> + switch (s->in) {
> >>>>>> + case EQUIRECTANGULAR:
> >>>>>> + in_transform = xyz_to_equirect;
> >>>>>> + err = 0;
> >>>>>> + wf = inlink->w;
> >>>>>> + hf = inlink->h;
> >>>>>> + break;
> >>>>>> + case CUBEMAP_3_2:
> >>>>>> + in_transform = xyz_to_cube3x2;
> >>>>>> + err = prepare_cube_in(ctx);
> >>>>>> + wf = inlink->w / 3.f * 4.f;
> >>>>>> + hf = inlink->h;
> >>>>>> + break;
> >>>>>> + case CUBEMAP_6_1:
> >>>>>> + in_transform = xyz_to_cube6x1;
> >>>>>> + err = prepare_cube_in(ctx);
> >>>>>> + wf = inlink->w / 3.f * 2.f;
> >>>>>> + hf = inlink->h * 2.f;
> >>>>>> + break;
> >>>>>> + case EQUIANGULAR:
> >>>>>> + in_transform = xyz_to_eac;
> >>>>>> + err = prepare_eac_in(ctx);
> >>>>>> + wf = inlink->w;
> >>>>>> + hf = inlink->h / 9.f * 8.f;
> >>>>>> + break;
> >>>>>> + case FLAT:
> >>>>>> + av_log(ctx, AV_LOG_ERROR, "Flat format is not accepted as
> >>>>>> input.\n");
> >>>>>> + return AVERROR(EINVAL);
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (err != 0) {
> >>>>>> + return err;
> >>>>>> + }
> >>>>>> +
> >>>>>> + switch (s->out) {
> >>>>>> + case EQUIRECTANGULAR:
> >>>>>> + out_transform = equirect_to_xyz;
> >>>>>> + err = 0;
> >>>>>> + w = roundf(wf);
> >>>>>> + h = roundf(hf);
> >>>>>> + break;
> >>>>>> + case CUBEMAP_3_2:
> >>>>>> + out_transform = cube3x2_to_xyz;
> >>>>>> + err = prepare_cube_out(ctx);
> >>>>>> + w = roundf(wf / 4.f * 3.f);
> >>>>>> + h = roundf(hf);
> >>>>>> + break;
> >>>>>> + case CUBEMAP_6_1:
> >>>>>> + out_transform = cube6x1_to_xyz;
> >>>>>> + err = prepare_cube_out(ctx);
> >>>>>> + w = roundf(wf / 2.f * 3.f);
> >>>>>> + h = roundf(hf / 2.f);
> >>>>>> + break;
> >>>>>> + case EQUIANGULAR:
> >>>>>> + out_transform = eac_to_xyz;
> >>>>>> + err = prepare_eac_out(ctx);
> >>>>>> + w = roundf(wf);
> >>>>>> + h = roundf(hf / 8.f * 9.f);
> >>>>>> + break;
> >>>>>> + case FLAT:
> >>>>>> + out_transform = flat_to_xyz;
> >>>>>> + err = prepare_flat_out(ctx);
> >>>>>> + w = roundf(wf * s->flat_range[0] / s->flat_range[1] / 2.f);
> >>>>>> + h = roundf(hf);
> >>>>>> + break;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (err != 0) {
> >>>>>> + return err;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (s->width > 0 && s->height > 0) {
> >>>>>> + w = s->width;
> >>>>>> + h = s->height;
> >>>>>> + }
> >>>>> If s->width/height are checked, should handle the case of no ture,
> >>>>> Else w/h may be used but not initialized.
> >>>>>
> >>>> Please try more hard to write english. I do not understand what is
> typed
> >>>> above.
> >>> If w and h don't have initial values at declaration. So they might
> never
> >>> get set if that last if statement evaluates to false.
> >>>
> >>> So he suggests to add an else case like
> >>>
> >>> if (s->width > 0 && s->height > 0) {
> >>> w = s->width;
> >>> h = s->height;
> >>> } else {
> >>> w = 0;
> >>> h = 0;
> >>> }
> >>>
> >>> or whatever values might make sense.
> >>>
> >> else case should have assert or return AVERROR_BUG.
> >
> > This "if" branch overrides default values with user values. Default
> values are calculated just above this code based on input/output formats.
> > So w and h do get initialized in any case.
>
> Hmm... what happens if (s->out != any case you have in the switch) and
> (s->width < 1 || s->height < 1)
>
> Maybe I misread, actually I just clarified what Li Zhong said.
>
It is covered, one might add simply assert to make sure sloppy developer
does not leave above code unchecked.
>
> -Thilo
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list