Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v4 1/4] avcodec/libjxldec: use internal AVFrame as buffered space
Date: Tue, 27 Jun 2023 22:08:36 +0200
Message-ID: <AS8P250MB07449C616A81C42E2F64C6578F27A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20230626154922.66550-2-leo.izen@gmail.com>

Leo Izen:
> Before this commit, the decoder erroneously assumes that the AVFrame
> passed to the receive_frame is the same one each time. Now it keeps an
> internal AVFrame to write into, and copies it over when it's done.
> ---
>  libavcodec/libjxldec.c | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
> index 50417bcb02..e516c29be5 100644
> --- a/libavcodec/libjxldec.c
> +++ b/libavcodec/libjxldec.c
> @@ -58,6 +58,7 @@ typedef struct LibJxlDecodeContext {
>      int64_t frame_duration;
>      int prev_is_last;
>      AVRational timebase;
> +    AVFrame *frame;
>  } LibJxlDecodeContext;
>  
>  static int libjxl_init_jxl_decoder(AVCodecContext *avctx)
> @@ -104,6 +105,9 @@ static av_cold int libjxl_decode_init(AVCodecContext *avctx)
>  
>      ctx->avpkt = avctx->internal->in_pkt;
>      ctx->pts = 0;
> +    ctx->frame = av_frame_alloc();
> +    if (!ctx->frame)
> +        return AVERROR(ENOMEM);
>  
>      return libjxl_init_jxl_decoder(avctx);
>  }
> @@ -351,10 +355,12 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>          size_t remaining;
>  
>          if (!pkt->size) {
> +            av_log(avctx, AV_LOG_DEBUG, "Fetching new packet\n");
>              av_packet_unref(pkt);
>              ret = ff_decode_get_packet(avctx, pkt);
>              if (ret < 0 && ret != AVERROR_EOF)
>                  return ret;
> +            av_log(avctx, AV_LOG_DEBUG, "Fetched packet of size: %d\n", pkt->size);

These logs have nothing to do with what the patch is supposed to do (and
they would belong in ff_decode_get_packet() anyway instead of in a
single user).

>              if (!pkt->size) {
>                  /* jret set by the last iteration of the loop */
>                  if (jret == JXL_DEC_NEED_MORE_INPUT) {
> @@ -389,10 +395,6 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>              return AVERROR_INVALIDDATA;
>          case JXL_DEC_NEED_MORE_INPUT:
>              av_log(avctx, AV_LOG_DEBUG, "NEED_MORE_INPUT event emitted\n");
> -            if (!pkt->size) {
> -                av_packet_unref(pkt);
> -                return AVERROR(EAGAIN);
> -            }
>              continue;
>          case JXL_DEC_BASIC_INFO:
>              av_log(avctx, AV_LOG_DEBUG, "BASIC_INFO event emitted\n");
> @@ -421,16 +423,18 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>              continue;
>          case JXL_DEC_COLOR_ENCODING:
>              av_log(avctx, AV_LOG_DEBUG, "COLOR_ENCODING event emitted\n");
> -            if ((ret = libjxl_color_encoding_event(avctx, frame)) < 0)
> +            if ((ret = libjxl_color_encoding_event(avctx, ctx->frame)) < 0)
>                  return ret;
>              continue;
>          case JXL_DEC_NEED_IMAGE_OUT_BUFFER:
>              av_log(avctx, AV_LOG_DEBUG, "NEED_IMAGE_OUT_BUFFER event emitted\n");
> -            if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
> +            ret = ff_get_buffer(avctx, ctx->frame, 0);
> +            if (ret < 0)
>                  return ret;
> -            ctx->jxl_pixfmt.align = frame->linesize[0];
> -            if (JxlDecoderSetImageOutBuffer(ctx->decoder, &ctx->jxl_pixfmt, frame->data[0], frame->buf[0]->size)
> -                != JXL_DEC_SUCCESS) {
> +            ctx->jxl_pixfmt.align = ctx->frame->linesize[0];
> +            if (JxlDecoderSetImageOutBuffer(ctx->decoder, &ctx->jxl_pixfmt,
> +                    ctx->frame->data[0], ctx->frame->buf[0]->size)
> +                    != JXL_DEC_SUCCESS) {
>                  av_log(avctx, AV_LOG_ERROR, "Bad libjxl dec need image out buffer event\n");
>                  return AVERROR_EXTERNAL;
>              }
> @@ -444,8 +448,8 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>          case JXL_DEC_FRAME:
>              av_log(avctx, AV_LOG_DEBUG, "FRAME event emitted\n");
>              if (!ctx->basic_info.have_animation || ctx->prev_is_last) {
> -                frame->pict_type = AV_PICTURE_TYPE_I;
> -                frame->flags |= AV_FRAME_FLAG_KEY;
> +                ctx->frame->pict_type = AV_PICTURE_TYPE_I;
> +                ctx->frame->flags |= AV_FRAME_FLAG_KEY;
>              }
>              if (ctx->basic_info.have_animation) {
>                  JxlFrameHeader header;
> @@ -464,20 +468,21 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>              /* full image is one frame, even if animated */
>              av_log(avctx, AV_LOG_DEBUG, "FULL_IMAGE event emitted\n");
>              if (ctx->iccp) {
> -                AVFrameSideData *sd = av_frame_new_side_data_from_buf(frame, AV_FRAME_DATA_ICC_PROFILE, ctx->iccp);
> +                AVFrameSideData *sd = av_frame_new_side_data_from_buf(ctx->frame, AV_FRAME_DATA_ICC_PROFILE, ctx->iccp);
>                  if (!sd)
>                      return AVERROR(ENOMEM);
>                  /* ownership is transfered, and it is not ref-ed */
>                  ctx->iccp = NULL;
>              }
>              if (avctx->pkt_timebase.num) {
> -                frame->pts = av_rescale_q(ctx->pts, ctx->timebase, avctx->pkt_timebase);
> -                frame->duration = av_rescale_q(ctx->frame_duration, ctx->timebase, avctx->pkt_timebase);
> +                ctx->frame->pts = av_rescale_q(ctx->pts, ctx->timebase, avctx->pkt_timebase);
> +                ctx->frame->duration = av_rescale_q(ctx->frame_duration, ctx->timebase, avctx->pkt_timebase);
>              } else {
> -                frame->pts = ctx->pts;
> -                frame->duration = ctx->frame_duration;
> +                ctx->frame->pts = ctx->pts;
> +                ctx->frame->duration = ctx->frame_duration;
>              }
>              ctx->pts += ctx->frame_duration;
> +            av_frame_move_ref(frame, ctx->frame);
>              return 0;
>          case JXL_DEC_SUCCESS:
>              av_log(avctx, AV_LOG_DEBUG, "SUCCESS event emitted\n");
> @@ -508,6 +513,7 @@ static av_cold int libjxl_decode_close(AVCodecContext *avctx)
>          JxlDecoderDestroy(ctx->decoder);
>      ctx->decoder = NULL;
>      av_buffer_unref(&ctx->iccp);
> +    av_frame_free(&ctx->frame);
>  
>      return 0;
>  }

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2023-06-27 20:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-26 15:49 [FFmpeg-devel] [PATCH v4 0/4] JPEG XL Parser Leo Izen
2023-06-26 15:49 ` [FFmpeg-devel] [PATCH v4 1/4] avcodec/libjxldec: use internal AVFrame as buffered space Leo Izen
2023-06-27 20:08   ` Andreas Rheinhardt [this message]
2023-06-26 15:49 ` [FFmpeg-devel] [PATCH v4 2/4] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
2023-06-27 22:58   ` James Almer
2023-06-27 23:18     ` Leo Izen
2023-06-27 23:20       ` James Almer
2023-06-26 15:49 ` [FFmpeg-devel] [PATCH v4 3/4] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse Leo Izen
2023-06-26 15:49 ` [FFmpeg-devel] [PATCH v4 4/4] fate/jpegxl_anim: add demuxer fate test for jpegxl_anim Leo Izen
2023-06-27 23:25   ` James Almer
2023-06-27 23:32     ` Leo Izen
2023-06-27 23:46       ` James Almer
2023-06-28  0:33         ` Leo Izen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AS8P250MB07449C616A81C42E2F64C6578F27A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \
    --to=andreas.rheinhardt@outlook.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git