| Message ID | 1554394133-25560-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | Accepted |
| Commit | 0a347ff4222345c88b93d7962a696f0e9aae7ca2 |
| Headers | show |
Jun Zhao: > From: Jun Zhao <barryjzhao@tencent.com> > > Fix memory leak after write trailer for #7827, only store a audio > packet whose buffer has size greater than zero in cur_audio_pkt. > > Thanks to Andreas Rheinhardt for the suggestions. > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > --- > libavformat/matroskaenc.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index b9f99c4..06f3aeb 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -2534,7 +2534,8 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) > // buffer an audio packet to ensure the packet containing the video > // keyframe's timecode is contained in the same cluster for WebM > if (codec_type == AVMEDIA_TYPE_AUDIO) { > - ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > + if (pkt->size > 0) > + ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > } else > ret = mkv_write_packet_internal(s, pkt, 0); > return ret; > Seems that I took quite a lot of time to write my commit message. I don't care which patch gets committed, but I presume you did run valgrind to make sure that it actually fixes #7827? - Andreas
On Fri, Apr 5, 2019 at 12:38 AM Andreas Rheinhardt via ffmpeg-devel < ffmpeg-devel@ffmpeg.org> wrote: > > Jun Zhao: > > From: Jun Zhao <barryjzhao@tencent.com> > > > > Fix memory leak after write trailer for #7827, only store a audio > > packet whose buffer has size greater than zero in cur_audio_pkt. > > > > Thanks to Andreas Rheinhardt for the suggestions. > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > --- > > libavformat/matroskaenc.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > > index b9f99c4..06f3aeb 100644 > > --- a/libavformat/matroskaenc.c > > +++ b/libavformat/matroskaenc.c > > @@ -2534,7 +2534,8 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) > > // buffer an audio packet to ensure the packet containing the video > > // keyframe's timecode is contained in the same cluster for WebM > > if (codec_type == AVMEDIA_TYPE_AUDIO) { > > - ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > > + if (pkt->size > 0) > > + ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > > } else > > ret = mkv_write_packet_internal(s, pkt, 0); > > return ret; > > > Seems that I took quite a lot of time to write my commit message. I > don't care which patch gets committed, but I presume you did run > valgrind to make sure that it actually fixes #7827? > > - Andreas Yes, I've run the valgrind for muxing with FLAC and other audio codec for this issue.
On Mon, Apr 8, 2019 at 9:09 AM mypopy@gmail.com <mypopy@gmail.com> wrote: > > > > On Fri, Apr 5, 2019 at 12:38 AM Andreas Rheinhardt via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: > > > > Jun Zhao: > > > From: Jun Zhao <barryjzhao@tencent.com> > > > > > > Fix memory leak after write trailer for #7827, only store a audio > > > packet whose buffer has size greater than zero in cur_audio_pkt. > > > > > > Thanks to Andreas Rheinhardt for the suggestions. > > > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > > --- > > > libavformat/matroskaenc.c | 3 ++- > > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > > > index b9f99c4..06f3aeb 100644 > > > --- a/libavformat/matroskaenc.c > > > +++ b/libavformat/matroskaenc.c > > > @@ -2534,7 +2534,8 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) > > > // buffer an audio packet to ensure the packet containing the video > > > // keyframe's timecode is contained in the same cluster for WebM > > > if (codec_type == AVMEDIA_TYPE_AUDIO) { > > > - ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > > > + if (pkt->size > 0) > > > + ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > > > } else > > > ret = mkv_write_packet_internal(s, pkt, 0); > > > return ret; > > > > > Seems that I took quite a lot of time to write my commit message. I > > don't care which patch gets committed, but I presume you did run > > valgrind to make sure that it actually fixes #7827? > > > > - Andreas > Yes, I've run the valgrind for muxing with FLAC and other audio codec for this issue. Will close the https://trac.ffmpeg.org/ticket/7827, Thanks
On Mon, Apr 8, 2019 at 3:29 PM mypopy@gmail.com <mypopy@gmail.com> wrote: > > On Mon, Apr 8, 2019 at 9:09 AM mypopy@gmail.com <mypopy@gmail.com> wrote: > > > > > > > > On Fri, Apr 5, 2019 at 12:38 AM Andreas Rheinhardt via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: > > > > > > Jun Zhao: > > > > From: Jun Zhao <barryjzhao@tencent.com> > > > > > > > > Fix memory leak after write trailer for #7827, only store a audio > > > > packet whose buffer has size greater than zero in cur_audio_pkt. > > > > > > > > Thanks to Andreas Rheinhardt for the suggestions. > > > > > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > > > --- > > > > libavformat/matroskaenc.c | 3 ++- > > > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > > > > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > > > > index b9f99c4..06f3aeb 100644 > > > > --- a/libavformat/matroskaenc.c > > > > +++ b/libavformat/matroskaenc.c > > > > @@ -2534,7 +2534,8 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) > > > > // buffer an audio packet to ensure the packet containing the video > > > > // keyframe's timecode is contained in the same cluster for WebM > > > > if (codec_type == AVMEDIA_TYPE_AUDIO) { > > > > - ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > > > > + if (pkt->size > 0) > > > > + ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); > > > > } else > > > > ret = mkv_write_packet_internal(s, pkt, 0); > > > > return ret; > > > > > > > Seems that I took quite a lot of time to write my commit message. I > > > don't care which patch gets committed, but I presume you did run > > > valgrind to make sure that it actually fixes #7827? > > > > > > - Andreas > > Yes, I've run the valgrind for muxing with FLAC and other audio codec for this issue. > > Will close the https://trac.ffmpeg.org/ticket/7827, Thanks Pushed (updated the commit message from Andreas Rheinhardt's patch and add Andreas Rheinhardt to Signed-off-by list). Thanks
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index b9f99c4..06f3aeb 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -2534,7 +2534,8 @@ static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt) // buffer an audio packet to ensure the packet containing the video // keyframe's timecode is contained in the same cluster for WebM if (codec_type == AVMEDIA_TYPE_AUDIO) { - ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); + if (pkt->size > 0) + ret = av_packet_ref(&mkv->cur_audio_pkt, pkt); } else ret = mkv_write_packet_internal(s, pkt, 0); return ret;