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

Soft Works softworkz at hotmail.com
Mon Dec 13 09:10:15 EET 2021



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Xiang,
> Haihao
> Sent: Monday, December 13, 2021 6:55 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
> 
> 
> > > -----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.
> 
> av_frame_copy_side_data() is used to copy the side data from frame0 to
> frame1.
> However in av_frame_copy_side_data(), av_frame_new_side_data_from_buf() is
> used
> to create a new side data of a given type for frame1. If frame1 has already
> haven a side data of a given type before copying the side data, frame1 will
> have
> two side data with the same type after copying. Some av functions, e.g.
> av_frame_get_side_data() only takes the fist side data of a given type into
> account, so it is possible the side data from frame0 won't be used in the
> final
> result.
> 
> I think av_frame_remove_side_data() should be used on frame1 before copying
> the
> side data?

Yes, you are right, that makes sense. I need to update the patch anyway
as Anton has taken vase of the 1/2 part. 

I'll post a new patch shortly.

Thanks a lot for reviewing,
softworkz




More information about the ffmpeg-devel mailing list