From patchwork Wed Jan 3 11:25:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthieu Bouron X-Patchwork-Id: 7102 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.79.195 with SMTP id r64csp15774217jad; Wed, 3 Jan 2018 03:23:37 -0800 (PST) X-Google-Smtp-Source: ACJfBotPYjSkCKnxXp3qty+aX6Gpq8UyreV/qcWaHNBA+3m+K8vN9lEwQCSEZd5fk77UsbtrKhFS X-Received: by 10.28.196.194 with SMTP id u185mr1207271wmf.133.1514978617352; Wed, 03 Jan 2018 03:23:37 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1514978617; cv=none; d=google.com; s=arc-20160816; b=Y5pw9RWmDf/U/TTMetBOUNxpiA8nrHoTrB88FFkFDAj3g8fLqWMurypvEIvsUi795V O3TK58byLdxU6eDR36+ElSM+OEkUHQ8CYDfZyROjIbEgZ6dFN2TsZKjosFiWvetq0h8Y lYZlAIr1y+eZ6JwSbKQpjbokDTosRz2E+fsOzA1kC79CwDjnCEyp9kQvU/pPfKAyvkrD 67rCGGkeNx24i+73smRgvCCnfbYvE5/p7mOWa+NZlOgOo1mJvhbwQFPUo1dIIGFDT8hW gapIVoYnjYbyJvArPH2YyQhS5QdUzL/6nu7Z42L8FoChNTreIxAlGjDtAMPu8QxpRF8f tdqA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :in-reply-to:content-disposition:mime-version:references:message-id :to:from:date:dkim-signature:delivered-to:arc-authentication-results; bh=1HWf0S0z1B3W9ATcnuMMqN4rhDxrYAn1JYtEaGflXfk=; b=qwXJplBEtXZOoDXnL+lRqBnTGdAkBb8q+ZB6PGLZ2pY6ihBG8UPOvrMRGrhF0t24j0 EDzPhbnPMpQBmnd/UKwFkd+/zeCNKtTdeyBeIDHi8v3YDrjQoP05Dt1Xanpx9SuDTw+v Z5PSRupAtLtOQgSGidAPaH/IE1xnSiKJEYVv+loeY/yNnXKpQbXiMZdgZ8tKNM0FLghL d4G7Lzpt63G0+AiNwLJSU32ZghIt9WqJ8LGY63bRIZA78k18Q4pnmWJT4JWavlY0HHh3 UfLWaeIQR6TTZEPYUFWoklcHapRsepj3+rlciw5r8GexCHXE/xO7J3FLR7qyTBaDmbSM 80aQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=M5Hsu0B/; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id l3si636542wrg.58.2018.01.03.03.23.36; Wed, 03 Jan 2018 03:23:37 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=M5Hsu0B/; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C5F61688319; Wed, 3 Jan 2018 13:23:18 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr0-f172.google.com (mail-wr0-f172.google.com [209.85.128.172]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DAB3D680612 for ; Wed, 3 Jan 2018 13:23:12 +0200 (EET) Received: by mail-wr0-f172.google.com with SMTP id w68so1202253wrc.10 for ; Wed, 03 Jan 2018 03:23:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=3p/ZWlucUJBafCTepDv+bd7e5eyqaYZ7G8A2M+hfDBg=; b=M5Hsu0B/egSoikeRRA8HFrbuCUHqRlOzvDa75/QBvbpUU6jFIYjeMfe69faPnNrHmr cyuF7H6y2bBAMy0Ki6v4XWTr+7GlWbSsfJd44LHdwNwkVms427CIy3FFh+u51Jb9cvQR 4ngUwSgnahMjNJkoCwKY5tvsDCDAMblvXjtHwUNaiC17RK/5cEwUuvoyLk9Q3xzHEWQd GCWCMwCqwiylVU/W1RyCCujElzDdYNhXl2o3URik1HM/kpJUsrXWqUXmA13epDa84NY+ 7JAbDDwuDl9v9RViQWJrLMU+3P2z9HlCamgb7w9OkLASplUuDp3bQ+gyYHWiomM2iV/B I67g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3p/ZWlucUJBafCTepDv+bd7e5eyqaYZ7G8A2M+hfDBg=; b=exct8c3Ebq3BwcyD+eBISYoiyTJJBylXsRBs9ftKZvR8SjzdepyvQpJY8r7oMfKPdn AKZJohShO44rMPcP2KY75xlSpCQGtBCC3QmpN/36ODxJTJojIUcT4EYsIBB6ua1zpkRh xsHIYVXauBNLKkawJnPQPbg2Vq0GbOHHdU4bbm97X+YD71zAHOIXhpgh+b9kyufuO68m ThZQ5Ig+e9jGsrbQ39vWyvIYQ3Rl7bEu7nxCPKi5yPdlkBGAeVF6K3WDrMiIamtqPKtp 4zFQAaZyCbdDns39GD7x/h45Ax8e1BaU8sr9uKMto8F84nNEYtfa4WLVxvziEehDuDsB y3ow== X-Gm-Message-State: AKGB3mKFidexayND9jYbSMQjWOA1wE1cGUgrLpU235ZnbMgnVtrTAhFt 3QN2U4S/ebR/BNB8awi+1n43iQ== X-Received: by 10.223.173.184 with SMTP id w53mr1205277wrc.144.1514978608563; Wed, 03 Jan 2018 03:23:28 -0800 (PST) Received: from tsuri.lan (AMontsouris-653-1-231-218.w86-212.abo.wanadoo.fr. [86.212.90.218]) by smtp.gmail.com with ESMTPSA id c53sm2443478wrg.10.2018.01.03.03.23.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 03 Jan 2018 03:23:27 -0800 (PST) Date: Wed, 3 Jan 2018 12:25:30 +0100 From: Matthieu Bouron To: Aman Gupta Message-ID: <20180103112530.GA21692@tsuri.lan> References: <20171229003653.66604-1-ffmpeg@tmm1.net> <20171229013314.77974-1-ffmpeg@tmm1.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171229013314.77974-1-ffmpeg@tmm1.net> User-Agent: Mutt/1.9.2 (2017-12-15) Subject: Re: [FFmpeg-devel] [PATCH v2] libavcodec/mediacodecdec: switch to new decoder api X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Aman Gupta , ffmpeg-devel@ffmpeg.org Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On Thu, Dec 28, 2017 at 05:33:14PM -0800, Aman Gupta wrote: > From: Aman Gupta > > Using the new API gives the decoder the ability to produce > N frames per input packet. This is particularly useful with > mpeg2 decoders on some android devices, which automatically > deinterlace video and produce one frame per field. > > Signed-off-by: Aman Gupta > --- > libavcodec/mediacodecdec.c | 77 +++++++++++++++++++++++++++------------------- > 1 file changed, 45 insertions(+), 32 deletions(-) Hi, Thanks for the patch. I'm attaching a new version of your patch. The new version fixes some issues that appear while seeking or when the decoder reaches EOF. The following comments correspond to the changes I made in the new patch (except for a few minor cosmetics like = {0}; -> = { 0 };). > > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c > index b698ceaef9..90497c64da 100644 > --- a/libavcodec/mediacodecdec.c > +++ b/libavcodec/mediacodecdec.c > @@ -31,6 +31,7 @@ > #include "libavutil/pixfmt.h" > > #include "avcodec.h" > +#include "decode.h" > #include "h264_parse.h" > #include "hevc_parse.h" > #include "hwaccel.h" > @@ -424,29 +425,13 @@ static int mediacodec_process_data(AVCodecContext *avctx, AVFrame *frame, > return ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, pkt); > } > > -static int mediacodec_decode_frame(AVCodecContext *avctx, void *data, > - int *got_frame, AVPacket *avpkt) > +static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame) > { > MediaCodecH264DecContext *s = avctx->priv_data; > - AVFrame *frame = data; > int ret; > - > - /* buffer the input packet */ > - if (avpkt->size) { > - AVPacket input_pkt = { 0 }; > - > - if (av_fifo_space(s->fifo) < sizeof(input_pkt)) { > - ret = av_fifo_realloc2(s->fifo, > - av_fifo_size(s->fifo) + sizeof(input_pkt)); > - if (ret < 0) > - return ret; > - } > - > - ret = av_packet_ref(&input_pkt, avpkt); > - if (ret < 0) > - return ret; > - av_fifo_generic_write(s->fifo, &input_pkt, sizeof(input_pkt), NULL); > - } > + int got_frame = 0; > + int is_eof = 0; > + AVPacket pkt = {0}; > > /* > * MediaCodec.flush() discards both input and output buffers, thus we > @@ -470,26 +455,51 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data, > */ > if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { > if (!ff_mediacodec_dec_flush(avctx, s->ctx)) { > - return avpkt->size; > + return 0; > } > + return AVERROR(EAGAIN); You should only return AVERROR(EAGAIN) if ff_mediacodec_dec_flush returns 0: if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { if (!ff_mediacodec_dec_flush(avctx, s->ctx)) { return AVERROR(EAGAIN); } } > + } > + > + ret = ff_decode_get_packet(avctx, &pkt); > + if (ret == AVERROR_EOF) > + is_eof = 1; > + else if (ret == AVERROR(EAGAIN)) > + ; // no input packet, but fallthrough to check for pending frames > + else if (ret < 0) > + return ret; > + > + /* buffer the input packet */ > + if (pkt.size) { > + if (av_fifo_space(s->fifo) < sizeof(pkt)) { > + ret = av_fifo_realloc2(s->fifo, > + av_fifo_size(s->fifo) + sizeof(pkt)); > + if (ret < 0) { > + av_packet_unref(&pkt); > + return ret; > + } > + } > + av_fifo_generic_write(s->fifo, &pkt, sizeof(pkt), NULL); > } > > /* process buffered data */ > - while (!*got_frame) { > + while (!got_frame) { > /* prepare the input data */ > if (s->buffered_pkt.size <= 0) { > av_packet_unref(&s->buffered_pkt); > > /* no more data */ > if (av_fifo_size(s->fifo) < sizeof(AVPacket)) { > - return avpkt->size ? avpkt->size : > - ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, avpkt); > + AVPacket null_pkt = {0}; > + if (is_eof) > + return ff_mediacodec_dec_decode(avctx, s->ctx, frame, > + &got_frame, &null_pkt); Returning the return value of ff_mediacodec_dec_decode is not valid. It should be something like: ret = ff_mediacodec_dec_decode(avctx, s->ctx, frame, &got_frame, &null_pkt); if (ret < 0) return ret; else if (got_frame) return 0; else return AVERROR_EOF; > + return AVERROR(EAGAIN); > } > > av_fifo_generic_read(s->fifo, &s->buffered_pkt, sizeof(s->buffered_pkt), NULL); > } > > - ret = mediacodec_process_data(avctx, frame, got_frame, &s->buffered_pkt); > + ret = mediacodec_process_data(avctx, frame, &got_frame, &s->buffered_pkt); > if (ret < 0) > return ret; > > @@ -497,7 +507,10 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data, > s->buffered_pkt.data += ret; > } > > - return avpkt->size; > + if (got_frame) > + return 0; > + > + return AVERROR(EAGAIN); got_frame is always 1 here, returning 0 should be enough. [...] Let me know if the attached updated patch works for you. Best regards, diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c index 3460478f43..94bff039a6 100644 --- a/libavcodec/mediacodecdec.c +++ b/libavcodec/mediacodecdec.c @@ -31,6 +31,7 @@ #include "libavutil/pixfmt.h" #include "avcodec.h" +#include "decode.h" #include "h264_parse.h" #include "hevc_parse.h" #include "hwaccel.h" @@ -442,29 +443,13 @@ static int mediacodec_process_data(AVCodecContext *avctx, AVFrame *frame, return ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, pkt); } -static int mediacodec_decode_frame(AVCodecContext *avctx, void *data, - int *got_frame, AVPacket *avpkt) +static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame) { MediaCodecH264DecContext *s = avctx->priv_data; - AVFrame *frame = data; int ret; - - /* buffer the input packet */ - if (avpkt->size) { - AVPacket input_pkt = { 0 }; - - if (av_fifo_space(s->fifo) < sizeof(input_pkt)) { - ret = av_fifo_realloc2(s->fifo, - av_fifo_size(s->fifo) + sizeof(input_pkt)); - if (ret < 0) - return ret; - } - - ret = av_packet_ref(&input_pkt, avpkt); - if (ret < 0) - return ret; - av_fifo_generic_write(s->fifo, &input_pkt, sizeof(input_pkt), NULL); - } + int got_frame = 0; + int is_eof = 0; + AVPacket pkt = { 0 }; /* * MediaCodec.flush() discards both input and output buffers, thus we @@ -488,26 +473,57 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data, */ if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) { if (!ff_mediacodec_dec_flush(avctx, s->ctx)) { - return avpkt->size; + return AVERROR(EAGAIN); } } + ret = ff_decode_get_packet(avctx, &pkt); + if (ret == AVERROR_EOF) + is_eof = 1; + else if (ret == AVERROR(EAGAIN)) + ; /* no input packet, but fallthrough to check for pending frames */ + else if (ret < 0) + return ret; + + /* buffer the input packet */ + if (pkt.size) { + if (av_fifo_space(s->fifo) < sizeof(pkt)) { + ret = av_fifo_realloc2(s->fifo, + av_fifo_size(s->fifo) + sizeof(pkt)); + if (ret < 0) { + av_packet_unref(&pkt); + return ret; + } + } + av_fifo_generic_write(s->fifo, &pkt, sizeof(pkt), NULL); + } + /* process buffered data */ - while (!*got_frame) { + while (!got_frame) { /* prepare the input data */ if (s->buffered_pkt.size <= 0) { av_packet_unref(&s->buffered_pkt); /* no more data */ if (av_fifo_size(s->fifo) < sizeof(AVPacket)) { - return avpkt->size ? avpkt->size : - ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, avpkt); + AVPacket null_pkt = { 0 }; + if (is_eof) { + ret = ff_mediacodec_dec_decode(avctx, s->ctx, frame, + &got_frame, &null_pkt); + if (ret < 0) + return ret; + else if (got_frame) + return 0; + else + return AVERROR_EOF; + } + return AVERROR(EAGAIN); } av_fifo_generic_read(s->fifo, &s->buffered_pkt, sizeof(s->buffered_pkt), NULL); } - ret = mediacodec_process_data(avctx, frame, got_frame, &s->buffered_pkt); + ret = mediacodec_process_data(avctx, frame, &got_frame, &s->buffered_pkt); if (ret < 0) return ret; @@ -515,7 +531,7 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data, s->buffered_pkt.data += ret; } - return avpkt->size; + return 0; } static void mediacodec_decode_flush(AVCodecContext *avctx) @@ -555,7 +571,7 @@ AVCodec ff_h264_mediacodec_decoder = { .id = AV_CODEC_ID_H264, .priv_data_size = sizeof(MediaCodecH264DecContext), .init = mediacodec_decode_init, - .decode = mediacodec_decode_frame, + .receive_frame = mediacodec_receive_frame, .flush = mediacodec_decode_flush, .close = mediacodec_decode_close, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE, @@ -574,7 +590,7 @@ AVCodec ff_hevc_mediacodec_decoder = { .id = AV_CODEC_ID_HEVC, .priv_data_size = sizeof(MediaCodecH264DecContext), .init = mediacodec_decode_init, - .decode = mediacodec_decode_frame, + .receive_frame = mediacodec_receive_frame, .flush = mediacodec_decode_flush, .close = mediacodec_decode_close, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE, @@ -593,7 +609,7 @@ AVCodec ff_mpeg2_mediacodec_decoder = { .id = AV_CODEC_ID_MPEG2VIDEO, .priv_data_size = sizeof(MediaCodecH264DecContext), .init = mediacodec_decode_init, - .decode = mediacodec_decode_frame, + .receive_frame = mediacodec_receive_frame, .flush = mediacodec_decode_flush, .close = mediacodec_decode_close, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE, @@ -611,7 +627,7 @@ AVCodec ff_mpeg4_mediacodec_decoder = { .id = AV_CODEC_ID_MPEG4, .priv_data_size = sizeof(MediaCodecH264DecContext), .init = mediacodec_decode_init, - .decode = mediacodec_decode_frame, + .receive_frame = mediacodec_receive_frame, .flush = mediacodec_decode_flush, .close = mediacodec_decode_close, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE, @@ -629,7 +645,7 @@ AVCodec ff_vp8_mediacodec_decoder = { .id = AV_CODEC_ID_VP8, .priv_data_size = sizeof(MediaCodecH264DecContext), .init = mediacodec_decode_init, - .decode = mediacodec_decode_frame, + .receive_frame = mediacodec_receive_frame, .flush = mediacodec_decode_flush, .close = mediacodec_decode_close, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE, @@ -647,7 +663,7 @@ AVCodec ff_vp9_mediacodec_decoder = { .id = AV_CODEC_ID_VP9, .priv_data_size = sizeof(MediaCodecH264DecContext), .init = mediacodec_decode_init, - .decode = mediacodec_decode_frame, + .receive_frame = mediacodec_receive_frame, .flush = mediacodec_decode_flush, .close = mediacodec_decode_close, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,