Re: [FFmpeg-devel] [PATCH v1] avcodec/libvpxenc: fix potential memory leak.
On Tue, Feb 16, 2021 at 4:17 PM Wonkap Jang
wrote:
>
> On Tue, Feb 16, 2021 at 1:02 PM James Zern
> wrote:
> [...]
> >
> > >
> > > while ((en2 =
> > av_dict_get(ctx->vpx_ref_frame_config, "", en2, AV_DICT_IGNORE_SUFFIX))) {
> > > @@ -1604,6 +1605,8 @@ static int vpx_encode(AVCodecContext *avctx,
> > AVPacket *pkt,
> > > en2->key, en2->value);
> > > }
> > >
> > > +if (ctx->vpx_ref_frame_config)
> >
> > This check is unnecessary.
> >
> [WJ] if parsing failed at first try without allocating anything? I saw
> examples checking for it.
>
The call checks the validity of the pointer, though
libavutil/tests/dict.c doesn't look to explicitly test that condition.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1] avcodec/libvpxenc: fix potential memory leak.
On Tue, Feb 16, 2021 at 1:02 PM James Zern
wrote:
> Hi,
>
> On Mon, Feb 15, 2021 at 10:37 PM Wonkap Jang
> wrote:
> >
> > While parsing ref_frame_config, AVdictionary needs to be manually
> > deallocated.
> > ---
> > libavcodec/libvpxenc.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > index 284cb9a108..941c3fdd88 100644
> > --- a/libavcodec/libvpxenc.c
> > +++ b/libavcodec/libvpxenc.c
> > @@ -1595,6 +1595,7 @@ static int vpx_encode(AVCodecContext *avctx,
> AVPacket *pkt,
> > if (en) {
> > if (avctx->codec_id == AV_CODEC_ID_VP9) {
> > AVDictionaryEntry* en2 = NULL;
> > +ctx->vpx_ref_frame_config = NULL;
> > av_dict_parse_string(>vpx_ref_frame_config,
> en->value, "=", ":", 0);
>
> Is there value in allowing a partial parse of the string? This should
> at least issue a warning if the call fails; vpx_ref_frame_config
> should be freed in either case.
>
[WJ] I need to do it since I need to get all parameters into an
AVDictionary object, and recurse through them later.
>
> >
> > while ((en2 =
> av_dict_get(ctx->vpx_ref_frame_config, "", en2, AV_DICT_IGNORE_SUFFIX))) {
> > @@ -1604,6 +1605,8 @@ static int vpx_encode(AVCodecContext *avctx,
> AVPacket *pkt,
> > en2->key, en2->value);
> > }
> >
> > +if (ctx->vpx_ref_frame_config)
>
> This check is unnecessary.
>
[WJ] if parsing failed at first try without allocating anything? I saw
examples checking for it.
> > +av_dict_free(>vpx_ref_frame_config);
> > codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG,
> (int *)>ref_frame_config);
> > } else {
> > av_log(avctx, AV_LOG_WARNING,
> > --
> > 2.30.0.478.g8a0d178c01-goog
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Hi James, my comments are in-line.
I'll send out the fixed version soon.
Thanks,
Wonkap
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1] avcodec/libvpxenc: fix potential memory leak.
Hi,
On Mon, Feb 15, 2021 at 10:37 PM Wonkap Jang
wrote:
>
> While parsing ref_frame_config, AVdictionary needs to be manually
> deallocated.
> ---
> libavcodec/libvpxenc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 284cb9a108..941c3fdd88 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -1595,6 +1595,7 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket
> *pkt,
> if (en) {
> if (avctx->codec_id == AV_CODEC_ID_VP9) {
> AVDictionaryEntry* en2 = NULL;
> +ctx->vpx_ref_frame_config = NULL;
> av_dict_parse_string(>vpx_ref_frame_config,
> en->value, "=", ":", 0);
Is there value in allowing a partial parse of the string? This should
at least issue a warning if the call fails; vpx_ref_frame_config
should be freed in either case.
>
> while ((en2 = av_dict_get(ctx->vpx_ref_frame_config, "",
> en2, AV_DICT_IGNORE_SUFFIX))) {
> @@ -1604,6 +1605,8 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket
> *pkt,
> en2->key, en2->value);
> }
>
> +if (ctx->vpx_ref_frame_config)
This check is unnecessary.
> +av_dict_free(>vpx_ref_frame_config);
> codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG, (int
> *)>ref_frame_config);
> } else {
> av_log(avctx, AV_LOG_WARNING,
> --
> 2.30.0.478.g8a0d178c01-goog
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1] avcodec/libvpxenc: fix potential memory leak.
While parsing ref_frame_config, AVdictionary needs to be manually
deallocated.
---
libavcodec/libvpxenc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 284cb9a108..941c3fdd88 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -1595,6 +1595,7 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket
*pkt,
if (en) {
if (avctx->codec_id == AV_CODEC_ID_VP9) {
AVDictionaryEntry* en2 = NULL;
+ctx->vpx_ref_frame_config = NULL;
av_dict_parse_string(>vpx_ref_frame_config,
en->value, "=", ":", 0);
while ((en2 = av_dict_get(ctx->vpx_ref_frame_config, "",
en2, AV_DICT_IGNORE_SUFFIX))) {
@@ -1604,6 +1605,8 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket
*pkt,
en2->key, en2->value);
}
+if (ctx->vpx_ref_frame_config)
+av_dict_free(>vpx_ref_frame_config);
codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG, (int
*)>ref_frame_config);
} else {
av_log(avctx, AV_LOG_WARNING,
--
2.30.0.478.g8a0d178c01-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
