| Message ID | 20220110072917.2140-1-ffmpeg@haasn.xyz |
|---|---|
| State | Accepted |
| Commit | ecf09764dcfc4d90d2292d91f225bfd02211a17f |
| Headers | show |
| Series | [FFmpeg-devel] lavfi/vf_libplacebo: fix side data stripping logic | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
10 Jan 2022, 08:29 by ffmpeg@haasn.xyz: > From: Niklas Haas <git@haasn.dev> > > This was accidentally comparing s->colorspace against out->colorspace, > which is wrong - the intent was to compare in->colorspace against > out->colorspace. > > We also forgot to strip mastering metadata. Finally, the order is sort > of wrong - we should strip this side data *before* process_frames, > because otherwise it may end up being seen and used by libplacebo. > > Signed-off-by: Niklas Haas <git@haasn.dev> > --- > libavfilter/vf_libplacebo.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c > index 1386aaeb3a..31ae28ac38 100644 > --- a/libavfilter/vf_libplacebo.c > +++ b/libavfilter/vf_libplacebo.c > @@ -390,7 +390,7 @@ fail: > > static int filter_frame(AVFilterLink *link, AVFrame *in) > { > - int err, changed; > + int err, changed_csp; > AVFilterContext *ctx = link->dst; > LibplaceboContext *s = ctx->priv; > AVFilterLink *outlink = ctx->outputs[0]; > @@ -426,22 +426,25 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) > if (s->color_primaries >= 0) > out->color_primaries = s->color_primaries; > > - RET(process_frames(ctx, out, in)); > - > - int changed_csp = s->colorspace != out->colorspace || > - s->color_range != out->color_range || > - s->color_trc != out->color_trc || > - s->color_primaries != out->color_primaries; > + changed_csp = in->colorspace != out->colorspace || > + in->color_range != out->color_range || > + in->color_trc != out->color_trc || > + in->color_primaries != out->color_primaries; > > + /* Strip side data if no longer relevant */ > + if (changed_csp) { > + av_frame_remove_side_data(out, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA); > + av_frame_remove_side_data(out, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL); > + } > if (s->apply_dovi || changed_csp) { > - /* Strip side data if no longer relevant */ > av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_RPU_BUFFER); > av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_METADATA); > } > - > if (s->apply_filmgrain) > av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS); > > + RET(process_frames(ctx, out, in)); > + > av_frame_free(&in); > > return ff_filter_frame(outlink, out); > Pushed to both master and release/5.0, thanks.
diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c index 1386aaeb3a..31ae28ac38 100644 --- a/libavfilter/vf_libplacebo.c +++ b/libavfilter/vf_libplacebo.c @@ -390,7 +390,7 @@ fail: static int filter_frame(AVFilterLink *link, AVFrame *in) { - int err, changed; + int err, changed_csp; AVFilterContext *ctx = link->dst; LibplaceboContext *s = ctx->priv; AVFilterLink *outlink = ctx->outputs[0]; @@ -426,22 +426,25 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) if (s->color_primaries >= 0) out->color_primaries = s->color_primaries; - RET(process_frames(ctx, out, in)); - - int changed_csp = s->colorspace != out->colorspace || - s->color_range != out->color_range || - s->color_trc != out->color_trc || - s->color_primaries != out->color_primaries; + changed_csp = in->colorspace != out->colorspace || + in->color_range != out->color_range || + in->color_trc != out->color_trc || + in->color_primaries != out->color_primaries; + /* Strip side data if no longer relevant */ + if (changed_csp) { + av_frame_remove_side_data(out, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA); + av_frame_remove_side_data(out, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL); + } if (s->apply_dovi || changed_csp) { - /* Strip side data if no longer relevant */ av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_RPU_BUFFER); av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_METADATA); } - if (s->apply_filmgrain) av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS); + RET(process_frames(ctx, out, in)); + av_frame_free(&in); return ff_filter_frame(outlink, out);