Message ID | 20210106031234.13435-1-guangxin.xu@intel.com |
---|---|
State | Accepted |
Commit | 3bbe0c210b05fc6fbd7b1d4bbd8479db7f2cf957 |
Headers | show |
Series | [FFmpeg-devel] avcodec/qsv_enc: do not reuse enc_ctrl from previous frames | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On Wed, 2021-01-06 at 11:12 +0800, Xu Guangxin wrote: > fixes #8857 > > If we do not clear the enc_ctrl, we will reuse previous frames' data like > FrameType. > --- > libavcodec/qsvenc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > index 2bd2a56227..94473c4eab 100644 > --- a/libavcodec/qsvenc.c > +++ b/libavcodec/qsvenc.c > @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) > while (cur) { > if (cur->used && !cur->surface.Data.Locked) { > free_encoder_ctrl_payloads(&cur->enc_ctrl); > + //do not reuse enc_ctrl from previous frame > + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); > if (cur->frame->format == AV_PIX_FMT_QSV) { > av_frame_unref(cur->frame); > } LGTM Thanks Haihao
On Wed, 2021-01-06 at 06:53 +0000, Xiang, Haihao wrote: > On Wed, 2021-01-06 at 11:12 +0800, Xu Guangxin wrote: > > fixes #8857 > > > > If we do not clear the enc_ctrl, we will reuse previous frames' data like > > FrameType. > > --- > > libavcodec/qsvenc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > > index 2bd2a56227..94473c4eab 100644 > > --- a/libavcodec/qsvenc.c > > +++ b/libavcodec/qsvenc.c > > @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) > > while (cur) { > > if (cur->used && !cur->surface.Data.Locked) { > > free_encoder_ctrl_payloads(&cur->enc_ctrl); > > + //do not reuse enc_ctrl from previous frame > > + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); > > if (cur->frame->format == AV_PIX_FMT_QSV) { > > av_frame_unref(cur->frame); > > } > > LGTM Could someone help to merge this patch ? https://github.com/Intel-Media-SDK/MediaSDK/issues/2776 was fixed by this patchtoo. Thanks Haihao
On 1/6/2021 12:12 AM, Xu Guangxin wrote: > fixes #8857 > > If we do not clear the enc_ctrl, we will reuse previous frames' data like FrameType. > --- > libavcodec/qsvenc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > index 2bd2a56227..94473c4eab 100644 > --- a/libavcodec/qsvenc.c > +++ b/libavcodec/qsvenc.c > @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) > while (cur) { > if (cur->used && !cur->surface.Data.Locked) { > free_encoder_ctrl_payloads(&cur->enc_ctrl); > + //do not reuse enc_ctrl from previous frame > + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); I assume cur->enc_ctrl.ExtParam is not set, right? Otherwise this memset could lead to leaks. > if (cur->frame->format == AV_PIX_FMT_QSV) { > av_frame_unref(cur->frame); > } >
On Thu, 2021-09-09 at 12:32 -0300, James Almer wrote: > On 1/6/2021 12:12 AM, Xu Guangxin wrote: > > fixes #8857 > > > > If we do not clear the enc_ctrl, we will reuse previous frames' data like > > FrameType. > > --- > > libavcodec/qsvenc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > > index 2bd2a56227..94473c4eab 100644 > > --- a/libavcodec/qsvenc.c > > +++ b/libavcodec/qsvenc.c > > @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) > > while (cur) { > > if (cur->used && !cur->surface.Data.Locked) { > > free_encoder_ctrl_payloads(&cur->enc_ctrl); > > + //do not reuse enc_ctrl from previous frame > > + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); > > I assume cur->enc_ctrl.ExtParam is not set, right? Otherwise this memset > could lead to leaks. Right, it is not set. > > > if (cur->frame->format == AV_PIX_FMT_QSV) { > > av_frame_unref(cur->frame); > > } > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 9/9/2021 12:41 PM, Xiang, Haihao wrote: > On Thu, 2021-09-09 at 12:32 -0300, James Almer wrote: >> On 1/6/2021 12:12 AM, Xu Guangxin wrote: >>> fixes #8857 >>> >>> If we do not clear the enc_ctrl, we will reuse previous frames' data like >>> FrameType. >>> --- >>> libavcodec/qsvenc.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c >>> index 2bd2a56227..94473c4eab 100644 >>> --- a/libavcodec/qsvenc.c >>> +++ b/libavcodec/qsvenc.c >>> @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) >>> while (cur) { >>> if (cur->used && !cur->surface.Data.Locked) { >>> free_encoder_ctrl_payloads(&cur->enc_ctrl); >>> + //do not reuse enc_ctrl from previous frame >>> + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); >> >> I assume cur->enc_ctrl.ExtParam is not set, right? Otherwise this memset >> could lead to leaks. > > Right, it is not set. Applied then.
On Thu, 2021-09-09 at 12:54 -0300, James Almer wrote: > On 9/9/2021 12:41 PM, Xiang, Haihao wrote: > > On Thu, 2021-09-09 at 12:32 -0300, James Almer wrote: > > > On 1/6/2021 12:12 AM, Xu Guangxin wrote: > > > > fixes #8857 > > > > > > > > If we do not clear the enc_ctrl, we will reuse previous frames' data > > > > like > > > > FrameType. > > > > --- > > > > libavcodec/qsvenc.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > > > > index 2bd2a56227..94473c4eab 100644 > > > > --- a/libavcodec/qsvenc.c > > > > +++ b/libavcodec/qsvenc.c > > > > @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) > > > > while (cur) { > > > > if (cur->used && !cur->surface.Data.Locked) { > > > > free_encoder_ctrl_payloads(&cur->enc_ctrl); > > > > + //do not reuse enc_ctrl from previous frame > > > > + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); > > > > > > I assume cur->enc_ctrl.ExtParam is not set, right? Otherwise this memset > > > could lead to leaks. > > > > Right, it is not set. > > Applied then. Thanks James
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index 2bd2a56227..94473c4eab 100644 --- a/libavcodec/qsvenc.c +++ b/libavcodec/qsvenc.c @@ -1249,6 +1249,8 @@ static void clear_unused_frames(QSVEncContext *q) while (cur) { if (cur->used && !cur->surface.Data.Locked) { free_encoder_ctrl_payloads(&cur->enc_ctrl); + //do not reuse enc_ctrl from previous frame + memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl)); if (cur->frame->format == AV_PIX_FMT_QSV) { av_frame_unref(cur->frame); }