[FFmpeg-devel] [PATCH 2/2] ffmpeg: use avcodec_copy_context before avcodec_open2 and open both ctxs

[Lukasz Marek]

On 3 December 2014 at 03:15, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Wed, Dec 03, 2014 at 02:02:21AM +0100, Lukasz Marek wrote:
> > On 03.12.2014 01:53, Michael Niedermayer wrote:
> > >On Wed, Dec 03, 2014 at 01:39:12AM +0100, Lukasz Marek wrote:
> > >>On 03.12.2014 00:35, Michael Niedermayer wrote:
> > >>>On Tue, Dec 02, 2014 at 07:27:33PM +0100, Lukasz Marek wrote:
> > >>>>avcodec_copy_context make dest context unopened.
> > >>>>ffmpeg treats it as opened.
> > >>>>
> > >>>>Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> > >>>>---
> > >>>>  ffmpeg.c | 29 ++++++++++++++++++++---------
> > >>>>  1 file changed, 20 insertions(+), 9 deletions(-)
> > >>>>
> > >>>>diff --git a/ffmpeg.c b/ffmpeg.c
> > >>>>index b44401f..4fa9f1e 100644
> > >>>>--- a/ffmpeg.c
> > >>>>+++ b/ffmpeg.c
> > >>>>@@ -2932,6 +2932,7 @@ static int transcode_init(void)
> > >>>>          if (ost->encoding_needed) {
> > >>>>              AVCodec      *codec = ost->enc;
> > >>>>              AVCodecContext *dec = NULL;
> > >>>>+            AVDictionary   *opt_copy = NULL;
> > >>>>
> > >>>>              if ((ist = get_input_stream(ost)))
> > >>>>                  dec = ist->dec_ctx;
> > >>>>@@ -2949,13 +2950,24 @@ static int transcode_init(void)
> > >>>>                  av_dict_set(&ost->encoder_opts, "threads", "auto",
> 0);
> > >>>>              av_dict_set(&ost->encoder_opts,
> "side_data_only_packets", "1", 0);
> > >>>>
> > >>>>-            if ((ret = avcodec_open2(ost->enc_ctx, codec,
> &ost->encoder_opts)) < 0) {
> > >>>>+            ret = avcodec_copy_context(ost->st->codec,
> ost->enc_ctx);
> > >>>>+            if (ret < 0) {
> > >>>>+                av_log(NULL, AV_LOG_FATAL,
> > >>>>+                       "Error initializing the output stream codec
> context.\n");
> > >>>>+                exit_program(1);
> > >>>>+            }
> > >>>>+
> > >>>>+            av_dict_copy(&opt_copy, ost->encoder_opts, 0);
> > >>>>+            if ((ret = avcodec_open2(ost->enc_ctx, codec,
> &ost->encoder_opts)) < 0 ||
> > >>>>+                (ret = avcodec_open2(ost->st->codec, codec,
> &opt_copy)) < 0) {
> > >>>
> > >>>if the encoder is opened twice it would print any info or warnings
> > >>>twice. libxvid would open 2pass files twice potentially i think
> > >>>
> > >>>maybe coded_frame could just be allocated somewhere before use
> > >>>in ffmpeg raw and leaving the copy as it is in ffmpeg ?
> > >>
> > >>I'm not familiar with ffmpeg.c code closely; is ost->st->codec
> > >>context used by ffmpeg for anything? I guess not (but im not sure).
> > >>If so, I think is all working because of that. You have two contexts
> > >>that share some pointers (like mentioned coded_frame). Of cource it
> > >>can be reallocated here, but this code is still in contradiction
> > >>with doxy of avcodec_copy_context that states dest context should be
> > >>treat as  not opened. FFmpeg uses it as opened.
> > >
> > >where does ffmpeg treat the context as open ?
> >
> > I saw this coded_frame allocated in init function of codecs, so I
> > assumed that.
> >
> > >i know of the coded_frame case but thats only used to transfer some
> > >flags into the muxer and isnt really a pretty way to do it but
> > >for that simply allocating the frame so the flags can be set
> > >should work
> > >i mean you can probably just check if its NULL and allocate
> > >before the few flags are set in it
> >
> > So basically, I can just alloc it in avcodec_copy_context in case it
> > is present in src object? It is working, but I thought it is wrong
> > way to solve it.
>
> I suspect that would lead to some memleak, also it doesnt feel
> right.
>

Yes, probably would require some cleaning somewhere too.


> Ive removed the troubblesome coded_frame use in ffmpeg.c, it served
> no purpose anymore
> does that solve the issue?
>

Yes. Test is not crushing anymore. Thanks.
This patch can be dropped then.