On Mon, Feb 23, 2015 at 07:51:30PM +0100, wm4 wrote:
> On Mon, 23 Feb 2015 19:11:58 +0100
> Michael Niedermayer <michae...@gmx.at> wrote:
>
> > On Mon, Feb 23, 2015 at 06:49:36PM +0100, Michael Niedermayer wrote:
> > > On Mon, Feb 23, 2015 at 06:15:15PM +0100, wm4 wrote:
> > > > On Mon, 23 Feb 2015 17:19:16 +0100
> > > > Michael Niedermayer <michae...@gmx.at> wrote:
> > > >
> > > > > This should allow simplifying many encoders
> > > > >
> > > > > Signed-off-by: Michael Niedermayer <michae...@gmx.at>
> > > > > ---
> > > > > libavcodec/utils.c | 12 ++++++++++++
> > > > > 1 file changed, 12 insertions(+)
> > > > >
> > > > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > > > > index c5e6300..c6d588a 100644
> > > > > --- a/libavcodec/utils.c
> > > > > +++ b/libavcodec/utils.c
> > > > > @@ -1695,6 +1695,16 @@ int attribute_align_arg
> > > > > avcodec_open2(AVCodecContext *avctx, const AVCodec *code
> > > > > av_assert0(*(const AVClass **)avctx->priv_data ==
> > > > > codec->priv_class);
> > > > > }
> > > > >
> > > > > + if (av_codec_is_encoder(avctx->codec) &&
> > > > > + avctx->codec_type == AVMEDIA_TYPE_VIDEO &&
> > > > > + !avctx->coded_frame) {
> > > > > + avctx->coded_frame = av_frame_alloc();
> > > > > + if (avctx->coded_frame) {
> > > > > + av_assert0(avctx->coded_frame->key_frame);
> > > > > + avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > end:
> > > > > ff_unlock_avcodec();
> > > > > if (options) {
> > > > > @@ -2852,6 +2862,8 @@ av_cold int avcodec_close(AVCodecContext *avctx)
> > > > > av_freep(&avctx->internal->hwaccel_priv_data);
> > > > >
> > > > > av_freep(&avctx->internal);
> > > > > + if (av_codec_is_encoder(avctx->codec))
> > > > > + av_frame_free(&avctx->coded_frame);
> > > >
> > > > This isn't the same condition. You might free the frame, even if you
> > > > didn't allocate it.
> > >
> > > fixed locally by seting a internal variable so the condition is
> > > also not duplicated
> > >
> > >
> > > >
> > > > What's the value in allocating the frame anyway? Seems like it
> > > > increases the memory management pain levels by 200%.
> > >
> > > if its good or not is entirely for the lib uses to decide, nothing
> > > in FFmpeg should need it
> > > i just would like to avoid having this allocation duplicated in half
> > > the encoders
> >
> > so IMHO encoders which do not export anything in their coded_frame
> > should either be able to leave it NULL or have it handled by some
> > generic code that does not need to be duplicated in each encoder
>
> Can you explain more? Why do some encoders need it, and some not?coded_frame is used to export information from encoders, encoders should generally not need it themselfs [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
