On Wed, Apr 20, 2016 at 05:08:04PM +0200, wm4 wrote:
> On Wed, 20 Apr 2016 16:54:30 +0200
> Michael Niedermayer <mich...@niedermayer.cc> wrote:
>
> > On Tue, Apr 19, 2016 at 11:49:11AM +0200, wm4 wrote:
[...]
> >
> > [...]
> > > @@ -3522,6 +3613,21 @@ typedef struct AVCodec {
> > > int (*decode)(AVCodecContext *, void *outdata, int *outdata_size,
> > > AVPacket *avpkt);
> > > int (*close)(AVCodecContext *);
> > > /**
> > > + * Decode/encode API with decoupled packet/frame dataflow. The API
> > > is the
> > > + * same as the avcodec_ prefixed APIs (avcodec_send_frame() etc.),
> > > except
> > > + * that:
> > > + * - never called if the codec is closed or the wrong type,
> > > + * - AVPacket parameter change side data is applied right before
> > > calling
> > > + * AVCodec->send_packet,
> > > + * - if AV_CODEC_CAP_DELAY is not set, drain packets or frames are
> > > never sent,
> > > + * - only one drain packet is ever passed down (until the next
> > > flush()),
> > > + * - a drain AVPacket is always NULL (no need to check for
> > > avpkt->size).
> > > + */
> > > + int (*send_frame)(AVCodecContext *avctx, const AVFrame *frame);
> > > + int (*send_packet)(AVCodecContext *avctx, const AVPacket *avpkt);
> > > + int (*receive_frame)(AVCodecContext *avctx, AVFrame *frame);
> > > + int (*receive_packet)(AVCodecContext *avctx, AVPacket *avpkt);
> > > + /**
> > > * Flush buffers.
> > > * Will be called when seeking
> > > */
> >
> > This breaks ABI
> > the functions must be added after caps_internal or a major bump is
> > needed
> > caps_internal is accessed in libavformat/utils.c it seems
>
> Well, thees fields are clearly below this comment:
>
> /*****************************************************************
> * No fields below this line are part of the public API. They
> * may not be used outside of libavcodec and can be changed and
> * removed at will.
> * New public fields should be added right above.
> *****************************************************************
> */
>
> So if libavformat does this, it violates the ABI.
violates API yes :(
ill fix that in lavf if noone does before
>
> >
> >
> > [...]
> > > @@ -2655,6 +2692,243 @@ void avsubtitle_free(AVSubtitle *sub)
> > > memset(sub, 0, sizeof(AVSubtitle));
> > > }
> > >
> > > +static int do_decode(AVCodecContext *avctx, AVPacket *pkt)
> > > +{
> > > + int got_frame;
> > > + int ret;
> > > +
> > > + av_assert0(!avctx->internal->buffer_frame->buf[0]);
> > > +
> > > + if (!pkt)
> > > + pkt = avctx->internal->buffer_pkt;
> > > +
> > > + // This is the lesser evil. The field is for compatibility with
> > > legacy users
> > > + // of the legacy API, and users using the new API should not be
> > > forced to
> > > + // even know about this field.
> > > + avctx->refcounted_frames = 1;
> > > +
> >
> > > + // Some codecs (at least wma lossless) will crash when feeding drain
> > > packets
> > > + // after EOF was signaled.
> > > + if (avctx->internal->draining_done)
> > > + return AVERROR_EOF;
> >
> > maybe decoders ca be fixed to avoid this, can you add a TODO note or
> > something in the code maybe something like
> > // TODO: check if decoders can be changed to avoid this check and the
> > draining_done field
> >
> > that is unless this is needed for something else
>
> I looked at the wma code (at the time when I wrote the patch), and it
> didn't look simple. I concluded it's better not to change this, as
> strictly speaking you really shouldn't feed a decoder more drain
> packets after it's done. (Other decoders tend to exhibit weird behavior
> too.)
>
> The new API enforces more robustness, and the draining_done field is
> internal, so I'm fine with this for now.
ok
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
