[FFmpeg-devel] [PATCH v3 2/2] avcodec/vpp_qsv: Copy side data from input to output frame

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Xiang,
> Haihao
> Sent: Thursday, December 9, 2021 9:35 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/vpp_qsv: Copy side data
> from input to output frame
> 
> On Tue, 2021-12-07 at 12:39 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Anton
> > > Khirnov
> > > Sent: Tuesday, December 7, 2021 12:51 PM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/vpp_qsv: Copy side
> data
> > > from input to output frame
> > >
> > > Quoting Soft Works (2021-12-07 09:55:37)
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Anton
> > > > > Khirnov
> > > > > Sent: Tuesday, December 7, 2021 9:04 AM
> > > > > To: ffmpeg-devel at ffmpeg.org
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/vpp_qsv: Copy side
> > >
> > > data
> > > > > from input to output frame
> > > > >
> > > > > Quoting Soft Works (2021-12-03 08:58:31)
> > > > > > Signed-off-by: softworkz <softworkz at hotmail.com>
> > > > > > ---
> > > > > >  libavfilter/qsvvpp.c         |  5 +++++
> > > > > >  libavfilter/vf_overlay_qsv.c | 19 +++++++++++++++----
> > > > > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
> > > > > > index d1218355c7..b291216292 100644
> > > > > > --- a/libavfilter/qsvvpp.c
> > > > > > +++ b/libavfilter/qsvvpp.c
> > > > > > @@ -849,6 +849,11 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s,
> > > > >
> > > > > AVFilterLink *inlink, AVFrame *picr
> > > > > >                  return AVERROR(EAGAIN);
> > > > > >              break;
> > > > > >          }
> > > > > > +
> > > > > > +        ret = av_frame_copy_side_data(out_frame->frame, in_frame-
> > > >
> > > > frame,
> > > > > 0);
> > > > > > +        if (ret < 0)
> > > > > > +            return ret;
> > > > > > +
> > > > > >          out_frame->frame->pts = av_rescale_q(out_frame-
> > > > > > surface.Data.TimeStamp,
> > > > > >                                               default_tb, outlink-
> > > > > > time_base);
> > > > > >
> > > > > > diff --git a/libavfilter/vf_overlay_qsv.c
> > >
> > > b/libavfilter/vf_overlay_qsv.c
> > > > > > index 7e76b39aa9..02518e020c 100644
> > > > > > --- a/libavfilter/vf_overlay_qsv.c
> > > > > > +++ b/libavfilter/vf_overlay_qsv.c
> > > > > > @@ -231,13 +231,24 @@ static int process_frame(FFFrameSync *fs)
> > > > > >  {
> > > > > >      AVFilterContext  *ctx = fs->parent;
> > > > > >      QSVOverlayContext  *s = fs->opaque;
> > > > > > +    AVFrame       *frame0 = NULL;
> > > > > >      AVFrame        *frame = NULL;
> > > > > > -    int               ret = 0, i;
> > > > > > +    int               ret = 0;
> > > > > >
> > > > > > -    for (i = 0; i < ctx->nb_inputs; i++) {
> > > > > > +    for (unsigned i = 0; i < ctx->nb_inputs; i++) {
> > > > > >          ret = ff_framesync_get_frame(fs, i, &frame, 0);
> > > > > > -        if (ret == 0)
> > > > > > -            ret = ff_qsvvpp_filter_frame(s->qsv, ctx->inputs[i],
> > >
> > > frame);
> > > > > > +
> > > > > > +        if (ret == 0) {
> > > > > > +            AVFrame *temp;
> > > > > > +
> > > > > > +            if (i == 0)
> > > > > > +                frame0 = frame;
> > > > > > +            else
> > > > > > +                ret = av_frame_copy_side_data(frame, frame0, 0);
> > > > > > +
> > > > > > +            ret = ret < 0 ? ret : ff_qsvvpp_filter_frame(s->qsv,
> ctx-
> > > > > > inputs[i], frame);
> > > > >
> > > > > I don't quite understand the ownership semantics here. This function
> > > > > does not free frame, so I assume ff_qsvvpp_filter_frame() takes
> > > > > ownership of it. That would mean you're not allowed to keep a pointer
> to
> > > > > it and access it later, because it might have already been freed.
> > > >
> > > > The filter is using framesync, which is taking care of the ownership.
> > > > ff_qsvvpp_filter_frame() clones or copies the frame, depending on case.
> > > > Other than with the normal overlay filter, the frame from input0 is
> > > > not used for output. But the regular overlay filter has established the
> > > > convention that side data from input0 is being kept at the output.
> > >
> > > Okay, if you're sure that framesync guarantees the frame remaining valid
> > > then I have no objections.
> > >
> > > But note that temp is unused and should be removed.
> >
> > OK, thanks. Let's see what Haihao says, he's closest to the subject at the
> > moment.
> 
> Is it possible that the frame from input1 and the frame from input0 have the
> same type of side data ? If yes, which one will take effect in the output
> frame
> ?

You can see in the code above that I'm copying the side data from frame0 
to frame1 before submitting frame1 to qsvvpp. So, it's always the side data from
frame0 that will be used.
This achieves the same behavior as the other overlay filters.

Thanks,
softworkz