| Message ID | 20180424205926.42027-1-ffmpeg@tmm1.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 24, 2018 at 2:11 PM, Jan Ekström <jeebjp@gmail.com> wrote: > On Tue, Apr 24, 2018 at 11:59 PM, Aman Gupta <ffmpeg@tmm1.net> wrote: > > From: Aman Gupta <aman@tmm1.net> > > > > --- > > libavcodec/mediacodecdec.c | 24 +++--------------------- > > libavcodec/mediacodecdec_common.c | 12 ++++++++++++ > > 2 files changed, 15 insertions(+), 21 deletions(-) > > > > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c > > index 0d4a853f07..e5d3e6a0af 100644 > > --- a/libavcodec/mediacodecdec.c > > +++ b/libavcodec/mediacodecdec.c > > @@ -419,27 +419,9 @@ static int mediacodec_receive_frame(AVCodecContext > *avctx, AVFrame *frame) > > MediaCodecH264DecContext *s = avctx->priv_data; > > int ret; > > > > - /* > > - * MediaCodec.flush() discards both input and output buffers, thus > we > > - * need to delay the call to this function until the user has > released or > > - * renderered the frames he retains. > > - * > > - * After we have buffered an input packet, check if the codec is in > the > > - * flushing state. If it is, we need to call > ff_mediacodec_dec_flush. > > - * > > - * ff_mediacodec_dec_flush returns 0 if the flush cannot be > performed on > > - * the codec (because the user retains frames). The codec stays in > the > > - * flushing state. > > - * > > - * ff_mediacodec_dec_flush returns 1 if the flush can actually be > > - * performed on the codec. The codec leaves the flushing state and > can > > - * process again packets. > > - * > > - * ff_mediacodec_dec_flush returns a negative value if an error has > > - * occurred. > > - * > > - */ > > - if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { > > + /* in delay_flush mode, wait until the user has released or rendered > > + all retained frames. */ > > + if (s->delay_flush && ff_mediacodec_dec_is_flushing(avctx, > s->ctx)) { > > if (!ff_mediacodec_dec_flush(avctx, s->ctx)) { > > return AVERROR(EAGAIN); > > } > > The commit message makes it sound as if you are only improving the > documentation, while you have now added an additional limitation here > (only check for flushing if delay_flush is true), so you might want to > detail on the "What & Why" in the commit message. Otherwise looks > alright. > There are no functional changes in this patch. Previously, ff_mediacodec_dec_is_flushing() would only ever be true if the user had set delay_flush. By adding that to the if statement, it makes it clearer that this part of the code is only in effect when that special option is set. Aman > > Jan >
On Tue, Apr 24, 2018 at 01:59:23PM -0700, Aman Gupta wrote: > From: Aman Gupta <aman@tmm1.net> > > --- > libavcodec/mediacodecdec.c | 24 +++--------------------- > libavcodec/mediacodecdec_common.c | 12 ++++++++++++ > 2 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c > index 0d4a853f07..e5d3e6a0af 100644 > --- a/libavcodec/mediacodecdec.c > +++ b/libavcodec/mediacodecdec.c > @@ -419,27 +419,9 @@ static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame) > MediaCodecH264DecContext *s = avctx->priv_data; > int ret; > > - /* > - * MediaCodec.flush() discards both input and output buffers, thus we > - * need to delay the call to this function until the user has released or > - * renderered the frames he retains. > - * > - * After we have buffered an input packet, check if the codec is in the > - * flushing state. If it is, we need to call ff_mediacodec_dec_flush. > - * > - * ff_mediacodec_dec_flush returns 0 if the flush cannot be performed on > - * the codec (because the user retains frames). The codec stays in the > - * flushing state. > - * > - * ff_mediacodec_dec_flush returns 1 if the flush can actually be > - * performed on the codec. The codec leaves the flushing state and can > - * process again packets. > - * > - * ff_mediacodec_dec_flush returns a negative value if an error has > - * occurred. > - * > - */ > - if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { > + /* in delay_flush mode, wait until the user has released or rendered NIT: In delay_flush mode, [...] > + all retained frames. */ > + if (s->delay_flush && ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { > if (!ff_mediacodec_dec_flush(avctx, s->ctx)) { > return AVERROR(EAGAIN); > } > diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c > index e59cf19aad..0c27624dea 100644 > --- a/libavcodec/mediacodecdec_common.c > +++ b/libavcodec/mediacodecdec_common.c > @@ -764,6 +764,18 @@ int ff_mediacodec_dec_receive(AVCodecContext *avctx, MediaCodecDecContext *s, > return AVERROR(EAGAIN); > } > > +/* > +* ff_mediacodec_dec_flush returns 0 if the flush cannot be performed on > +* the codec (because the user retains frames). The codec stays in the > +* flushing state. > +* > +* ff_mediacodec_dec_flush returns 1 if the flush can actually be > +* performed on the codec. The codec leaves the flushing state and can > +* process again packets. > +* > +* ff_mediacodec_dec_flush returns a negative value if an error has > +* occurred. > +*/ > int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s) > { > if (!s->surface || atomic_load(&s->refcount) == 1) { > -- > 2.14.2 > LGTM.
diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c index 0d4a853f07..e5d3e6a0af 100644 --- a/libavcodec/mediacodecdec.c +++ b/libavcodec/mediacodecdec.c @@ -419,27 +419,9 @@ static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame) MediaCodecH264DecContext *s = avctx->priv_data; int ret; - /* - * MediaCodec.flush() discards both input and output buffers, thus we - * need to delay the call to this function until the user has released or - * renderered the frames he retains. - * - * After we have buffered an input packet, check if the codec is in the - * flushing state. If it is, we need to call ff_mediacodec_dec_flush. - * - * ff_mediacodec_dec_flush returns 0 if the flush cannot be performed on - * the codec (because the user retains frames). The codec stays in the - * flushing state. - * - * ff_mediacodec_dec_flush returns 1 if the flush can actually be - * performed on the codec. The codec leaves the flushing state and can - * process again packets. - * - * ff_mediacodec_dec_flush returns a negative value if an error has - * occurred. - * - */ - if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { + /* in delay_flush mode, wait until the user has released or rendered + all retained frames. */ + if (s->delay_flush && ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { if (!ff_mediacodec_dec_flush(avctx, s->ctx)) { return AVERROR(EAGAIN); } diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c index e59cf19aad..0c27624dea 100644 --- a/libavcodec/mediacodecdec_common.c +++ b/libavcodec/mediacodecdec_common.c @@ -764,6 +764,18 @@ int ff_mediacodec_dec_receive(AVCodecContext *avctx, MediaCodecDecContext *s, return AVERROR(EAGAIN); } +/* +* ff_mediacodec_dec_flush returns 0 if the flush cannot be performed on +* the codec (because the user retains frames). The codec stays in the +* flushing state. +* +* ff_mediacodec_dec_flush returns 1 if the flush can actually be +* performed on the codec. The codec leaves the flushing state and can +* process again packets. +* +* ff_mediacodec_dec_flush returns a negative value if an error has +* occurred. +*/ int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s) { if (!s->surface || atomic_load(&s->refcount) == 1) {
From: Aman Gupta <aman@tmm1.net> --- libavcodec/mediacodecdec.c | 24 +++--------------------- libavcodec/mediacodecdec_common.c | 12 ++++++++++++ 2 files changed, 15 insertions(+), 21 deletions(-)