| Message ID | 20200327095557.24069-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avformat/avidec: Don't reimplement ff_free_stream() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Andreas Rheinhardt: > Using ff_free_stream() makes the code more readable, more future-proof > (the old code freed AVCodecContexts and AVCodecParameters and its > substructures manually, so that there is a chance that there would be a > memleak for some time if new substructures were added) and reduces > code size. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/avidec.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index 00c3978b2d..ae343e732a 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -600,21 +600,8 @@ static int avi_read_header(AVFormatContext *s) > goto fail; > > ast = s->streams[0]->priv_data; > - av_freep(&s->streams[0]->codecpar->extradata); > - av_freep(&s->streams[0]->codecpar); > -#if FF_API_LAVF_AVCTX > -FF_DISABLE_DEPRECATION_WARNINGS > - av_freep(&s->streams[0]->codec); > -FF_ENABLE_DEPRECATION_WARNINGS > -#endif > - if (s->streams[0]->info) > - av_freep(&s->streams[0]->info->duration_error); > - av_freep(&s->streams[0]->info); > - if (s->streams[0]->internal) > - av_freep(&s->streams[0]->internal->avctx); > - av_freep(&s->streams[0]->internal); > - av_freep(&s->streams[0]); > - s->nb_streams = 0; > + st->priv_data = NULL; > + ff_free_stream(s, st); > if (CONFIG_DV_DEMUXER) { > avi->dv_demux = avpriv_dv_init_demux(s); > if (!avi->dv_demux) > Any comments on this patchset? If there are no objections, I'll apply it tomorrow. - Andreas
On 3/27/2020 6:55 AM, Andreas Rheinhardt wrote: > Using ff_free_stream() makes the code more readable, more future-proof > (the old code freed AVCodecContexts and AVCodecParameters and its > substructures manually, so that there is a chance that there would be a > memleak for some time if new substructures were added) and reduces > code size. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/avidec.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index 00c3978b2d..ae343e732a 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -600,21 +600,8 @@ static int avi_read_header(AVFormatContext *s) > goto fail; > > ast = s->streams[0]->priv_data; > - av_freep(&s->streams[0]->codecpar->extradata); > - av_freep(&s->streams[0]->codecpar); > -#if FF_API_LAVF_AVCTX > -FF_DISABLE_DEPRECATION_WARNINGS > - av_freep(&s->streams[0]->codec); > -FF_ENABLE_DEPRECATION_WARNINGS > -#endif > - if (s->streams[0]->info) > - av_freep(&s->streams[0]->info->duration_error); > - av_freep(&s->streams[0]->info); > - if (s->streams[0]->internal) > - av_freep(&s->streams[0]->internal->avctx); > - av_freep(&s->streams[0]->internal); > - av_freep(&s->streams[0]); > - s->nb_streams = 0; > + st->priv_data = NULL; > + ff_free_stream(s, st); I stream_index guaranteed to be 0 here? Or in turn, was s->streams[0] correct in this chunk to begin with? > if (CONFIG_DV_DEMUXER) { > avi->dv_demux = avpriv_dv_init_demux(s); > if (!avi->dv_demux) >
James Almer: > On 3/27/2020 6:55 AM, Andreas Rheinhardt wrote: >> Using ff_free_stream() makes the code more readable, more future-proof >> (the old code freed AVCodecContexts and AVCodecParameters and its >> substructures manually, so that there is a chance that there would be a >> memleak for some time if new substructures were added) and reduces >> code size. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavformat/avidec.c | 17 ++--------------- >> 1 file changed, 2 insertions(+), 15 deletions(-) >> >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >> index 00c3978b2d..ae343e732a 100644 >> --- a/libavformat/avidec.c >> +++ b/libavformat/avidec.c >> @@ -600,21 +600,8 @@ static int avi_read_header(AVFormatContext *s) >> goto fail; >> >> ast = s->streams[0]->priv_data; >> - av_freep(&s->streams[0]->codecpar->extradata); >> - av_freep(&s->streams[0]->codecpar); >> -#if FF_API_LAVF_AVCTX >> -FF_DISABLE_DEPRECATION_WARNINGS >> - av_freep(&s->streams[0]->codec); >> -FF_ENABLE_DEPRECATION_WARNINGS >> -#endif >> - if (s->streams[0]->info) >> - av_freep(&s->streams[0]->info->duration_error); >> - av_freep(&s->streams[0]->info); >> - if (s->streams[0]->internal) >> - av_freep(&s->streams[0]->internal->avctx); >> - av_freep(&s->streams[0]->internal); >> - av_freep(&s->streams[0]); >> - s->nb_streams = 0; >> + st->priv_data = NULL; >> + ff_free_stream(s, st); > > I stream_index guaranteed to be 0 here? Or in turn, was s->streams[0] > correct in this chunk to begin with? > Yes. See lines 592-595: /* After some consideration -- I don't think we * have to support anything but DV in type1 AVIs. */ if (s->nb_streams != 1) goto fail; (I basically don't know DV, so I don't know how much additional work it would be to extend this. I also don't know if simply freeing the last stream (as ff_free_stream() does automatically) would mean that the above check could be removed. All I know is that the avpriv_dv_init_demux() will add a new stream to s.) - Andreas
Andreas Rheinhardt: > Using ff_free_stream() makes the code more readable, more future-proof > (the old code freed AVCodecContexts and AVCodecParameters and its > substructures manually, so that there is a chance that there would be a > memleak for some time if new substructures were added) and reduces > code size. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/avidec.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index 00c3978b2d..ae343e732a 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -600,21 +600,8 @@ static int avi_read_header(AVFormatContext *s) > goto fail; > > ast = s->streams[0]->priv_data; > - av_freep(&s->streams[0]->codecpar->extradata); > - av_freep(&s->streams[0]->codecpar); > -#if FF_API_LAVF_AVCTX > -FF_DISABLE_DEPRECATION_WARNINGS > - av_freep(&s->streams[0]->codec); > -FF_ENABLE_DEPRECATION_WARNINGS > -#endif > - if (s->streams[0]->info) > - av_freep(&s->streams[0]->info->duration_error); > - av_freep(&s->streams[0]->info); > - if (s->streams[0]->internal) > - av_freep(&s->streams[0]->internal->avctx); > - av_freep(&s->streams[0]->internal); > - av_freep(&s->streams[0]); > - s->nb_streams = 0; > + st->priv_data = NULL; > + ff_free_stream(s, st); > if (CONFIG_DV_DEMUXER) { > avi->dv_demux = avpriv_dv_init_demux(s); > if (!avi->dv_demux) > Applied the set. - Andreas
diff --git a/libavformat/avidec.c b/libavformat/avidec.c index 00c3978b2d..ae343e732a 100644 --- a/libavformat/avidec.c +++ b/libavformat/avidec.c @@ -600,21 +600,8 @@ static int avi_read_header(AVFormatContext *s) goto fail; ast = s->streams[0]->priv_data; - av_freep(&s->streams[0]->codecpar->extradata); - av_freep(&s->streams[0]->codecpar); -#if FF_API_LAVF_AVCTX -FF_DISABLE_DEPRECATION_WARNINGS - av_freep(&s->streams[0]->codec); -FF_ENABLE_DEPRECATION_WARNINGS -#endif - if (s->streams[0]->info) - av_freep(&s->streams[0]->info->duration_error); - av_freep(&s->streams[0]->info); - if (s->streams[0]->internal) - av_freep(&s->streams[0]->internal->avctx); - av_freep(&s->streams[0]->internal); - av_freep(&s->streams[0]); - s->nb_streams = 0; + st->priv_data = NULL; + ff_free_stream(s, st); if (CONFIG_DV_DEMUXER) { avi->dv_demux = avpriv_dv_init_demux(s); if (!avi->dv_demux)
Using ff_free_stream() makes the code more readable, more future-proof (the old code freed AVCodecContexts and AVCodecParameters and its substructures manually, so that there is a chance that there would be a memleak for some time if new substructures were added) and reduces code size. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/avidec.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)