| Message ID | 20221205133938.505-1-timo@rothenpieler.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/thread: add support for frame threading receive_frame based decoders | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Quoting Timo Rothenpieler (2022-12-05 14:39:37) > This is fairly basic and makes a lot of assumptions, but it works > for the most simple cases. > > For one, it only ever fetches exactly one packet per call to receive_frame. > Right now it's impossible for there to ever be more than one, but the API > allows for more, which might need handled in the future. > > It also basically translates the new API back to the old, since that's how > the frame threading code operates. Which feels backwards in regards to > the new API, but it was the path with least resistance in implementing this. If it only supports one packet to one frame, then it goes against the whole point of using the receive_frame API.
On 06/12/2022 15:37, Anton Khirnov wrote: > Quoting Timo Rothenpieler (2022-12-05 14:39:37) >> This is fairly basic and makes a lot of assumptions, but it works >> for the most simple cases. >> >> For one, it only ever fetches exactly one packet per call to receive_frame. >> Right now it's impossible for there to ever be more than one, but the API >> allows for more, which might need handled in the future. >> >> It also basically translates the new API back to the old, since that's how >> the frame threading code operates. Which feels backwards in regards to >> the new API, but it was the path with least resistance in implementing this. > > If it only supports one packet to one frame, then it goes against the > whole point of using the receive_frame API. Otherwise the entirety of pthread_frame.c would need rewritten from scratch. It has that assumption coded into it. It also raises question as to how it'd distribute those packets accross threads. Does it just try to read packets until it either runs out of ready threads or encounters EAGAIN? Or does it read until EAGAIN and then passes all those to the same threads?
Quoting Timo Rothenpieler (2022-12-06 15:39:50) > On 06/12/2022 15:37, Anton Khirnov wrote: > > Quoting Timo Rothenpieler (2022-12-05 14:39:37) > >> This is fairly basic and makes a lot of assumptions, but it works > >> for the most simple cases. > >> > >> For one, it only ever fetches exactly one packet per call to receive_frame. > >> Right now it's impossible for there to ever be more than one, but the API > >> allows for more, which might need handled in the future. > >> > >> It also basically translates the new API back to the old, since that's how > >> the frame threading code operates. Which feels backwards in regards to > >> the new API, but it was the path with least resistance in implementing this. > > > > If it only supports one packet to one frame, then it goes against the > > whole point of using the receive_frame API. > > Otherwise the entirety of pthread_frame.c would need rewritten from > scratch. It has that assumption coded into it. I told you on IRC I already have a mostly-finished branch that implements threading with receive_frame(), so I don't really understand what's the point of your writing this patch.
On 06/12/2022 15:43, Anton Khirnov wrote: > Quoting Timo Rothenpieler (2022-12-06 15:39:50) >> On 06/12/2022 15:37, Anton Khirnov wrote: >>> Quoting Timo Rothenpieler (2022-12-05 14:39:37) >>>> This is fairly basic and makes a lot of assumptions, but it works >>>> for the most simple cases. >>>> >>>> For one, it only ever fetches exactly one packet per call to receive_frame. >>>> Right now it's impossible for there to ever be more than one, but the API >>>> allows for more, which might need handled in the future. >>>> >>>> It also basically translates the new API back to the old, since that's how >>>> the frame threading code operates. Which feels backwards in regards to >>>> the new API, but it was the path with least resistance in implementing this. >>> >>> If it only supports one packet to one frame, then it goes against the >>> whole point of using the receive_frame API. >> >> Otherwise the entirety of pthread_frame.c would need rewritten from >> scratch. It has that assumption coded into it. > > I told you on IRC I already have a mostly-finished branch that > implements threading with receive_frame(), so I don't really understand > what's the point of your writing this patch. > You said it wasn't even in a state to be tested. Do you have a link to it? Happy to help finishing it up.
Quoting Timo Rothenpieler (2022-12-06 16:08:49) > On 06/12/2022 15:43, Anton Khirnov wrote: > > Quoting Timo Rothenpieler (2022-12-06 15:39:50) > >> On 06/12/2022 15:37, Anton Khirnov wrote: > >>> Quoting Timo Rothenpieler (2022-12-05 14:39:37) > >>>> This is fairly basic and makes a lot of assumptions, but it works > >>>> for the most simple cases. > >>>> > >>>> For one, it only ever fetches exactly one packet per call to receive_frame. > >>>> Right now it's impossible for there to ever be more than one, but the API > >>>> allows for more, which might need handled in the future. > >>>> > >>>> It also basically translates the new API back to the old, since that's how > >>>> the frame threading code operates. Which feels backwards in regards to > >>>> the new API, but it was the path with least resistance in implementing this. > >>> > >>> If it only supports one packet to one frame, then it goes against the > >>> whole point of using the receive_frame API. > >> > >> Otherwise the entirety of pthread_frame.c would need rewritten from > >> scratch. It has that assumption coded into it. > > > > I told you on IRC I already have a mostly-finished branch that > > implements threading with receive_frame(), so I don't really understand > > what's the point of your writing this patch. > > > > You said it wasn't even in a state to be tested. > Do you have a link to it? Happy to help finishing it up. Sure, see branch 'thread_receive' in git://git.khirnov.net/libav IIRC it was only missing some allocations for frames or packets in AVCodecInternal.
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 6be2d3d6ed..72a8253aae 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -577,7 +577,11 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame) av_assert0(!frame->buf[0]); if (codec->cb_type == FF_CODEC_CB_TYPE_RECEIVE_FRAME) { - ret = codec->cb.receive_frame(avctx, frame); + if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) + ret = ff_thread_receive_frame(avctx, frame); + else + ret = codec->cb.receive_frame(avctx, frame); + if (ret != AVERROR(EAGAIN)) av_packet_unref(avci->last_pkt_props); } else diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index df82a4125f..8f704e35d3 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -92,6 +92,7 @@ typedef struct PerThreadContext { AVCodecContext *avctx; ///< Context used to decode packets passed to this thread. AVPacket *avpkt; ///< Input packet (for decoding) or output (for encoding). + int avpkt_read; ///< Indicates if the packet has been read for this recv_frame call already. AVFrame *frame; ///< Output frame (for decoding) or input (for encoding). int got_frame; ///< The output of got_picture_ptr from the last avcodec_decode_video() call. @@ -237,8 +238,14 @@ FF_ENABLE_DEPRECATION_WARNINGS } av_frame_unref(p->frame); - p->got_frame = 0; - p->result = codec->cb.decode(avctx, p->frame, &p->got_frame, p->avpkt); + if (codec->cb_type == FF_CODEC_CB_TYPE_RECEIVE_FRAME) { + p->avpkt_read = 0; + p->result = codec->cb.receive_frame(avctx, p->frame); + p->got_frame = !p->result; + } else { + p->got_frame = 0; + p->result = codec->cb.decode(avctx, p->frame, &p->got_frame, p->avpkt); + } if ((p->result < 0 || !p->got_frame) && p->frame->buf[0]) ff_thread_release_buffer(avctx, p->frame); @@ -621,6 +628,58 @@ finish: return err; } +int ff_thread_receive_frame(AVCodecContext *avctx, AVFrame *frame) +{ + AVPacket *const avpkt = avctx->internal->in_pkt; + int got_picture = 0; + int draining = 0; + int ret; + + av_packet_unref(avpkt); + ret = ff_decode_get_packet(avctx, avpkt); + if (ret < 0 && ret != AVERROR_EOF) + return ret; + draining = ret == AVERROR_EOF; + + ret = ff_thread_decode_frame(avctx, frame, &got_picture, avpkt); + + if (ret == avpkt->size) { + if (got_picture) { + return 0; + } else if (draining) { + return AVERROR_EOF; + } + + return AVERROR(EAGAIN); + } else if (ret < 0) { + return ret; + } + + return AVERROR_BUG; +} + +int ff_thread_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt) +{ + PerThreadContext *p; + int err; + + if (!(avctx->active_thread_type & FF_THREAD_FRAME)) + return ff_decode_get_packet(avctx, pkt); + + p = avctx->internal->thread_ctx; + + if (p->avpkt_read) + return AVERROR(EAGAIN); + + err = av_packet_ref(pkt, p->avpkt); + if (err < 0) + return err; + + p->avpkt_read = 1; + + return 0; +} + void ff_thread_report_progress(ThreadFrame *f, int n, int field) { PerThreadContext *p; @@ -775,6 +834,7 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) av_freep(&ctx->slice_offset); av_buffer_unref(&ctx->internal->pool); + av_packet_free(&ctx->internal->in_pkt); av_freep(&ctx->internal); av_buffer_unref(&ctx->hw_frames_ctx); } @@ -826,6 +886,10 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, return AVERROR(ENOMEM); copy->internal->thread_ctx = p; + copy->internal->in_pkt = av_packet_alloc(); + if (!copy->internal->in_pkt) + return AVERROR(ENOMEM); + copy->delay = avctx->delay; if (codec->priv_data_size) { diff --git a/libavcodec/thread.h b/libavcodec/thread.h index d5673f25ea..76e7d44bd4 100644 --- a/libavcodec/thread.h +++ b/libavcodec/thread.h @@ -52,6 +52,15 @@ void ff_thread_flush(AVCodecContext *avctx); int ff_thread_decode_frame(AVCodecContext *avctx, AVFrame *picture, int *got_picture_ptr, AVPacket *avpkt); +/** + * Receive a new frame from a decoding thread. + * Returns the next available frame, or AVERROR(EAGAIN) if + * none is available. + * + * Parameters are the same as FFCodec.receive_frame. + */ +int ff_thread_receive_frame(AVCodecContext *avctx, AVFrame *frame); + /** * If the codec defines update_thread_context(), call this * when they are ready for the next thread to start decoding @@ -99,6 +108,14 @@ int ff_thread_get_buffer(AVCodecContext *avctx, AVFrame *f, int flags); */ void ff_thread_release_buffer(AVCodecContext *avctx, AVFrame *f); +/** + * Wrapper around ff_decode_get_packet() for frame-multithreaded codecs. + * Call this functions instead of ff_decode_get_packet(). + * + * Parameters are the same as ff_decode_get_packet(). + */ +int ff_thread_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt); + int ff_thread_init(AVCodecContext *s); int ff_slice_thread_execute_with_mainfunc(AVCodecContext *avctx, int (*action_func2)(AVCodecContext *c, void *arg, int jobnr, int threadnr),