* [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function
@ 2023-07-08 19:00 James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 2/3] avcodec/decode: check the output frame for discard samples with all decoders James Almer
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: James Almer @ 2023-07-08 19:00 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/decode.c | 182 +++++++++++++++++++++++---------------------
1 file changed, 94 insertions(+), 88 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 269633ce10..793ab975f6 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -289,69 +289,14 @@ static int64_t guess_correct_pts(AVCodecContext *ctx,
return pts;
}
-/*
- * The core of the receive_frame_wrapper for the decoders implementing
- * the simple API. Certain decoders might consume partial packets without
- * returning any output, so this function needs to be called in a loop until it
- * returns EAGAIN.
- **/
-static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
+static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
{
- AVCodecInternal *avci = avctx->internal;
- AVPacket *const pkt = avci->in_pkt;
- const FFCodec *const codec = ffcodec(avctx->codec);
- int got_frame, actual_got_frame;
- int ret;
-
- if (!pkt->data && !avci->draining) {
- av_packet_unref(pkt);
- ret = ff_decode_get_packet(avctx, pkt);
- if (ret < 0 && ret != AVERROR_EOF)
- return ret;
- }
-
- // Some codecs (at least wma lossless) will crash when feeding drain packets
- // after EOF was signaled.
- if (avci->draining_done)
- return AVERROR_EOF;
-
- if (!pkt->data &&
- !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
- avctx->active_thread_type & FF_THREAD_FRAME))
- return AVERROR_EOF;
-
- got_frame = 0;
-
- if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
- ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
- } else {
- ret = codec->cb.decode(avctx, frame, &got_frame, pkt);
-
- if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
- frame->pkt_dts = pkt->dts;
- if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
-#if FF_API_FRAME_PKT
-FF_DISABLE_DEPRECATION_WARNINGS
- if(!avctx->has_b_frames)
- frame->pkt_pos = pkt->pos;
-FF_ENABLE_DEPRECATION_WARNINGS
-#endif
- //FIXME these should be under if(!avctx->has_b_frames)
- /* get_buffer is supposed to set frame parameters */
- if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
- if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
- if (!frame->width) frame->width = avctx->width;
- if (!frame->height) frame->height = avctx->height;
- if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
- }
- }
- }
- emms_c();
- actual_got_frame = got_frame;
+ AVCodecInternal *avci = avctx->internal;
+ int ret = 0;
if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
if (frame->flags & AV_FRAME_FLAG_DISCARD)
- got_frame = 0;
+ ret = AVERROR(EAGAIN);
} else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
uint8_t *side;
size_t side_size;
@@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
uint8_t skip_reason = 0;
uint8_t discard_reason = 0;
- if (ret >= 0 && got_frame) {
if (frame->format == AV_SAMPLE_FMT_NONE)
frame->format = avctx->sample_fmt;
- if (!frame->ch_layout.nb_channels) {
- int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
- if (ret2 < 0) {
- ret = ret2;
- got_frame = 0;
- }
- }
+ if (!frame->ch_layout.nb_channels)
+ ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
#if FF_API_OLD_CHANNEL_LAYOUT
FF_DISABLE_DEPRECATION_WARNINGS
if (!frame->channel_layout)
@@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
#endif
if (!frame->sample_rate)
frame->sample_rate = avctx->sample_rate;
- }
side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
if(side && side_size>=10) {
@@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
discard_reason = AV_RL8(side + 9);
}
- if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame &&
+ if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret &&
!(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples);
- got_frame = 0;
*discarded_samples += frame->nb_samples;
+ ret = AVERROR(EAGAIN);
}
- if (avci->skip_samples > 0 && got_frame &&
+ if (avci->skip_samples > 0 && !ret &&
!(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
if(frame->nb_samples <= avci->skip_samples){
- got_frame = 0;
*discarded_samples += frame->nb_samples;
avci->skip_samples -= frame->nb_samples;
av_log(avctx, AV_LOG_DEBUG, "skip whole frame, skip left: %d\n",
avci->skip_samples);
+ ret = AVERROR(EAGAIN);
} else {
av_samples_copy(frame->extended_data, frame->extended_data, 0, avci->skip_samples,
frame->nb_samples - avci->skip_samples, avctx->ch_layout.nb_channels, frame->format);
@@ -432,11 +370,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
}
- if (discard_padding > 0 && discard_padding <= frame->nb_samples && got_frame &&
+ if (discard_padding > 0 && discard_padding <= frame->nb_samples && !ret &&
!(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
if (discard_padding == frame->nb_samples) {
*discarded_samples += frame->nb_samples;
- got_frame = 0;
+ ret = AVERROR(EAGAIN);
} else {
if(avctx->pkt_timebase.num && avctx->sample_rate) {
int64_t diff_ts = av_rescale_q(frame->nb_samples - discard_padding,
@@ -452,7 +390,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
}
- if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && got_frame) {
+ if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && !ret) {
AVFrameSideData *fside = av_frame_new_side_data(frame, AV_FRAME_DATA_SKIP_SAMPLES, 10);
if (fside) {
AV_WL32(fside->data, avci->skip_samples);
@@ -464,15 +402,88 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
}
- if (!got_frame)
+ return ret;
+}
+
+/*
+ * The core of the receive_frame_wrapper for the decoders implementing
+ * the simple API. Certain decoders might consume partial packets without
+ * returning any output, so this function needs to be called in a loop until it
+ * returns EAGAIN.
+ **/
+static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
+{
+ AVCodecInternal *avci = avctx->internal;
+ AVPacket *const pkt = avci->in_pkt;
+ const FFCodec *const codec = ffcodec(avctx->codec);
+ int got_frame;
+ int consumed, ret;
+
+ if (!pkt->data && !avci->draining) {
+ av_packet_unref(pkt);
+ ret = ff_decode_get_packet(avctx, pkt);
+ if (ret < 0 && ret != AVERROR_EOF)
+ return ret;
+ }
+
+ // Some codecs (at least wma lossless) will crash when feeding drain packets
+ // after EOF was signaled.
+ if (avci->draining_done)
+ return AVERROR_EOF;
+
+ if (!pkt->data &&
+ !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
+ avctx->active_thread_type & FF_THREAD_FRAME))
+ return AVERROR_EOF;
+
+ got_frame = 0;
+
+ if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
+ consumed = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
+ } else {
+ consumed = codec->cb.decode(avctx, frame, &got_frame, pkt);
+
+ if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
+ frame->pkt_dts = pkt->dts;
+ if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
+#if FF_API_FRAME_PKT
+FF_DISABLE_DEPRECATION_WARNINGS
+ if(!avctx->has_b_frames)
+ frame->pkt_pos = pkt->pos;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+ //FIXME these should be under if(!avctx->has_b_frames)
+ /* get_buffer is supposed to set frame parameters */
+ if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
+ if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
+ if (!frame->width) frame->width = avctx->width;
+ if (!frame->height) frame->height = avctx->height;
+ if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
+ }
+ }
+ }
+ emms_c();
+
+ if (got_frame)
+ ret = discard_samples(avctx, frame, discarded_samples);
+ else
+ ret = AVERROR(EAGAIN);
+
+ if (consumed < 0)
+ ret = consumed;
+ if (consumed >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
+ consumed = pkt->size;
+
+ if (!got_frame || ret == AVERROR(EAGAIN))
av_frame_unref(frame);
- if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
- ret = pkt->size;
+ if (!ret)
+ av_assert0(frame->buf[0]);
+ if (ret == AVERROR(EAGAIN))
+ ret = 0;
- /* do not stop draining when actual_got_frame != 0 or ret < 0 */
- /* got_frame == 0 but actual_got_frame != 0 when frame is discarded */
- if (avci->draining && !actual_got_frame) {
+ /* do not stop draining when got_frame != 0 or ret < 0 */
+ if (avci->draining && !got_frame) {
if (ret < 0) {
/* prevent infinite loop if a decoder wrongly always return error on draining */
/* reasonable nb_errors_max = maximum b frames + thread count */
@@ -490,11 +501,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
}
- if (ret >= pkt->size || ret < 0) {
+ if (consumed >= pkt->size || ret < 0) {
av_packet_unref(pkt);
} else {
- int consumed = ret;
-
pkt->data += consumed;
pkt->size -= consumed;
pkt->pts = AV_NOPTS_VALUE;
@@ -509,10 +518,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
}
- if (got_frame)
- av_assert0(frame->buf[0]);
-
- return ret < 0 ? ret : 0;
+ return ret;
}
#if CONFIG_LCMS2
--
2.41.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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* [FFmpeg-devel] [PATCH 2/3] avcodec/decode: check the output frame for discard samples with all decoders
2023-07-08 19:00 [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function James Almer
@ 2023-07-08 19:00 ` James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for " James Almer
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: James Almer @ 2023-07-08 19:00 UTC (permalink / raw)
To: ffmpeg-devel
And not just those with the old decode() API.
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/decode.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 793ab975f6..49c8b7e0f4 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -594,8 +594,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) {
+ int64_t discarded_samples = 0;
ret = codec->cb.receive_frame(avctx, frame);
emms_c();
+ if (!ret)
+ ret = discard_samples(avctx, frame, &discarded_samples);
} else
ret = decode_simple_receive_frame(avctx, frame);
--
2.41.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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for all decoders
2023-07-08 19:00 [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 2/3] avcodec/decode: check the output frame for discard samples with all decoders James Almer
@ 2023-07-08 19:00 ` James Almer
2023-07-09 9:49 ` Anton Khirnov
2023-07-08 19:38 ` [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function Andreas Rheinhardt
2023-07-09 9:48 ` Anton Khirnov
3 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2023-07-08 19:00 UTC (permalink / raw)
To: ffmpeg-devel
And not just those with the old decode() API.
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/decode.c | 52 +++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 49c8b7e0f4..fe07e906c2 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -304,22 +304,6 @@ static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *disca
uint8_t skip_reason = 0;
uint8_t discard_reason = 0;
- if (frame->format == AV_SAMPLE_FMT_NONE)
- frame->format = avctx->sample_fmt;
- if (!frame->ch_layout.nb_channels)
- ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
-#if FF_API_OLD_CHANNEL_LAYOUT
-FF_DISABLE_DEPRECATION_WARNINGS
- if (!frame->channel_layout)
- frame->channel_layout = avctx->ch_layout.order == AV_CHANNEL_ORDER_NATIVE ?
- avctx->ch_layout.u.mask : 0;
- if (!frame->channels)
- frame->channels = avctx->ch_layout.nb_channels;
-FF_ENABLE_DEPRECATION_WARNINGS
-#endif
- if (!frame->sample_rate)
- frame->sample_rate = avctx->sample_rate;
-
side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
if(side && side_size>=10) {
avci->skip_samples = AV_RL32(side);
@@ -452,14 +436,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
frame->pkt_pos = pkt->pos;
FF_ENABLE_DEPRECATION_WARNINGS
#endif
- //FIXME these should be under if(!avctx->has_b_frames)
- /* get_buffer is supposed to set frame parameters */
- if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
- if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
- if (!frame->width) frame->width = avctx->width;
- if (!frame->height) frame->height = avctx->height;
- if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
- }
}
}
emms_c();
@@ -613,8 +589,34 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
}
if (!ret) {
- if (avctx->codec_type != AVMEDIA_TYPE_VIDEO)
+ if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
+ /* get_buffer is supposed to set frame parameters */
+ if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
+ if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
+ if (!frame->width) frame->width = avctx->width;
+ if (!frame->height) frame->height = avctx->height;
+ if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
+ }
+ } else {
+ if (frame->format == AV_SAMPLE_FMT_NONE)
+ frame->format = avctx->sample_fmt;
+ if (!frame->ch_layout.nb_channels)
+ ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+ if (!frame->channel_layout)
+ frame->channel_layout = avctx->ch_layout.order == AV_CHANNEL_ORDER_NATIVE ?
+ avctx->ch_layout.u.mask : 0;
+ if (!frame->channels)
+ frame->channels = avctx->ch_layout.nb_channels;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+ if (!frame->sample_rate)
+ frame->sample_rate = avctx->sample_rate;
+
frame->flags |= AV_FRAME_FLAG_KEY;
+ }
+
#if FF_API_FRAME_KEY
FF_DISABLE_DEPRECATION_WARNINGS
frame->key_frame = !!(frame->flags & AV_FRAME_FLAG_KEY);
--
2.41.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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function
2023-07-08 19:00 [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 2/3] avcodec/decode: check the output frame for discard samples with all decoders James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for " James Almer
@ 2023-07-08 19:38 ` Andreas Rheinhardt
2023-07-08 19:41 ` James Almer
2023-07-09 9:48 ` Anton Khirnov
3 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2023-07-08 19:38 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/decode.c | 182 +++++++++++++++++++++++---------------------
> 1 file changed, 94 insertions(+), 88 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 269633ce10..793ab975f6 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -289,69 +289,14 @@ static int64_t guess_correct_pts(AVCodecContext *ctx,
> return pts;
> }
>
> -/*
> - * The core of the receive_frame_wrapper for the decoders implementing
> - * the simple API. Certain decoders might consume partial packets without
> - * returning any output, so this function needs to be called in a loop until it
> - * returns EAGAIN.
> - **/
> -static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> +static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> {
> - AVCodecInternal *avci = avctx->internal;
> - AVPacket *const pkt = avci->in_pkt;
> - const FFCodec *const codec = ffcodec(avctx->codec);
> - int got_frame, actual_got_frame;
> - int ret;
> -
> - if (!pkt->data && !avci->draining) {
> - av_packet_unref(pkt);
> - ret = ff_decode_get_packet(avctx, pkt);
> - if (ret < 0 && ret != AVERROR_EOF)
> - return ret;
> - }
> -
> - // Some codecs (at least wma lossless) will crash when feeding drain packets
> - // after EOF was signaled.
> - if (avci->draining_done)
> - return AVERROR_EOF;
> -
> - if (!pkt->data &&
> - !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
> - avctx->active_thread_type & FF_THREAD_FRAME))
> - return AVERROR_EOF;
> -
> - got_frame = 0;
> -
> - if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
> - ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
> - } else {
> - ret = codec->cb.decode(avctx, frame, &got_frame, pkt);
> -
> - if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
> - frame->pkt_dts = pkt->dts;
> - if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> -#if FF_API_FRAME_PKT
> -FF_DISABLE_DEPRECATION_WARNINGS
> - if(!avctx->has_b_frames)
> - frame->pkt_pos = pkt->pos;
> -FF_ENABLE_DEPRECATION_WARNINGS
> -#endif
> - //FIXME these should be under if(!avctx->has_b_frames)
> - /* get_buffer is supposed to set frame parameters */
> - if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
> - if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
> - if (!frame->width) frame->width = avctx->width;
> - if (!frame->height) frame->height = avctx->height;
> - if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
> - }
> - }
> - }
> - emms_c();
> - actual_got_frame = got_frame;
> + AVCodecInternal *avci = avctx->internal;
> + int ret = 0;
>
> if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> if (frame->flags & AV_FRAME_FLAG_DISCARD)
> - got_frame = 0;
> + ret = AVERROR(EAGAIN);
> } else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
> uint8_t *side;
> size_t side_size;
> @@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
> uint8_t skip_reason = 0;
> uint8_t discard_reason = 0;
>
> - if (ret >= 0 && got_frame) {
> if (frame->format == AV_SAMPLE_FMT_NONE)
> frame->format = avctx->sample_fmt;
> - if (!frame->ch_layout.nb_channels) {
> - int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
> - if (ret2 < 0) {
> - ret = ret2;
> - got_frame = 0;
> - }
> - }
> + if (!frame->ch_layout.nb_channels)
> + ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
> #if FF_API_OLD_CHANNEL_LAYOUT
> FF_DISABLE_DEPRECATION_WARNINGS
> if (!frame->channel_layout)
> @@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
> #endif
> if (!frame->sample_rate)
> frame->sample_rate = avctx->sample_rate;
> - }
>
> side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
> if(side && side_size>=10) {
> @@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
> discard_reason = AV_RL8(side + 9);
> }
>
> - if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame &&
> + if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret &&
> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
> avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples);
> - got_frame = 0;
> *discarded_samples += frame->nb_samples;
> + ret = AVERROR(EAGAIN);
> }
>
> - if (avci->skip_samples > 0 && got_frame &&
> + if (avci->skip_samples > 0 && !ret &&
> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
> if(frame->nb_samples <= avci->skip_samples){
> - got_frame = 0;
> *discarded_samples += frame->nb_samples;
> avci->skip_samples -= frame->nb_samples;
> av_log(avctx, AV_LOG_DEBUG, "skip whole frame, skip left: %d\n",
> avci->skip_samples);
> + ret = AVERROR(EAGAIN);
> } else {
> av_samples_copy(frame->extended_data, frame->extended_data, 0, avci->skip_samples,
> frame->nb_samples - avci->skip_samples, avctx->ch_layout.nb_channels, frame->format);
> @@ -432,11 +370,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
> }
> }
>
> - if (discard_padding > 0 && discard_padding <= frame->nb_samples && got_frame &&
> + if (discard_padding > 0 && discard_padding <= frame->nb_samples && !ret &&
> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
> if (discard_padding == frame->nb_samples) {
> *discarded_samples += frame->nb_samples;
> - got_frame = 0;
> + ret = AVERROR(EAGAIN);
> } else {
> if(avctx->pkt_timebase.num && avctx->sample_rate) {
> int64_t diff_ts = av_rescale_q(frame->nb_samples - discard_padding,
> @@ -452,7 +390,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> }
> }
>
> - if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && got_frame) {
> + if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && !ret) {
> AVFrameSideData *fside = av_frame_new_side_data(frame, AV_FRAME_DATA_SKIP_SAMPLES, 10);
> if (fside) {
> AV_WL32(fside->data, avci->skip_samples);
> @@ -464,15 +402,88 @@ FF_ENABLE_DEPRECATION_WARNINGS
> }
> }
>
> - if (!got_frame)
> + return ret;
> +}
> +
> +/*
> + * The core of the receive_frame_wrapper for the decoders implementing
> + * the simple API. Certain decoders might consume partial packets without
> + * returning any output, so this function needs to be called in a loop until it
> + * returns EAGAIN.
> + **/
> +static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> +{
> + AVCodecInternal *avci = avctx->internal;
> + AVPacket *const pkt = avci->in_pkt;
> + const FFCodec *const codec = ffcodec(avctx->codec);
> + int got_frame;
> + int consumed, ret;
> +
> + if (!pkt->data && !avci->draining) {
> + av_packet_unref(pkt);
> + ret = ff_decode_get_packet(avctx, pkt);
> + if (ret < 0 && ret != AVERROR_EOF)
> + return ret;
> + }
> +
> + // Some codecs (at least wma lossless) will crash when feeding drain packets
> + // after EOF was signaled.
> + if (avci->draining_done)
> + return AVERROR_EOF;
> +
> + if (!pkt->data &&
> + !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
> + avctx->active_thread_type & FF_THREAD_FRAME))
> + return AVERROR_EOF;
> +
> + got_frame = 0;
> +
> + if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
> + consumed = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
> + } else {
> + consumed = codec->cb.decode(avctx, frame, &got_frame, pkt);
> +
> + if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
> + frame->pkt_dts = pkt->dts;
> + if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> +#if FF_API_FRAME_PKT
> +FF_DISABLE_DEPRECATION_WARNINGS
> + if(!avctx->has_b_frames)
> + frame->pkt_pos = pkt->pos;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> + //FIXME these should be under if(!avctx->has_b_frames)
> + /* get_buffer is supposed to set frame parameters */
> + if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
> + if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
> + if (!frame->width) frame->width = avctx->width;
> + if (!frame->height) frame->height = avctx->height;
> + if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
> + }
> + }
> + }
> + emms_c();
> +
> + if (got_frame)
> + ret = discard_samples(avctx, frame, discarded_samples);
> + else
> + ret = AVERROR(EAGAIN);
> +
> + if (consumed < 0)
> + ret = consumed;
> + if (consumed >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
> + consumed = pkt->size;
> +
> + if (!got_frame || ret == AVERROR(EAGAIN))
> av_frame_unref(frame);
>
> - if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
> - ret = pkt->size;
> + if (!ret)
> + av_assert0(frame->buf[0]);
> + if (ret == AVERROR(EAGAIN))
> + ret = 0;
>
> - /* do not stop draining when actual_got_frame != 0 or ret < 0 */
> - /* got_frame == 0 but actual_got_frame != 0 when frame is discarded */
> - if (avci->draining && !actual_got_frame) {
> + /* do not stop draining when got_frame != 0 or ret < 0 */
> + if (avci->draining && !got_frame) {
> if (ret < 0) {
> /* prevent infinite loop if a decoder wrongly always return error on draining */
> /* reasonable nb_errors_max = maximum b frames + thread count */
> @@ -490,11 +501,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> }
> }
>
> - if (ret >= pkt->size || ret < 0) {
> + if (consumed >= pkt->size || ret < 0) {
> av_packet_unref(pkt);
> } else {
> - int consumed = ret;
> -
> pkt->data += consumed;
> pkt->size -= consumed;
> pkt->pts = AV_NOPTS_VALUE;
> @@ -509,10 +518,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> }
> }
>
> - if (got_frame)
> - av_assert0(frame->buf[0]);
> -
> - return ret < 0 ? ret : 0;
> + return ret;
> }
>
> #if CONFIG_LCMS2
Would using --diff-algorithm=minimal (or patience or histogram) produce
a more readable diff (i.e. one where the code that ends up in
discard_samples() is moved instead of everything except it)?
- Andreas
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function
2023-07-08 19:38 ` [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function Andreas Rheinhardt
@ 2023-07-08 19:41 ` James Almer
0 siblings, 0 replies; 11+ messages in thread
From: James Almer @ 2023-07-08 19:41 UTC (permalink / raw)
To: ffmpeg-devel
On 7/8/2023 4:38 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/decode.c | 182 +++++++++++++++++++++++---------------------
>> 1 file changed, 94 insertions(+), 88 deletions(-)
>>
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index 269633ce10..793ab975f6 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -289,69 +289,14 @@ static int64_t guess_correct_pts(AVCodecContext *ctx,
>> return pts;
>> }
>>
>> -/*
>> - * The core of the receive_frame_wrapper for the decoders implementing
>> - * the simple API. Certain decoders might consume partial packets without
>> - * returning any output, so this function needs to be called in a loop until it
>> - * returns EAGAIN.
>> - **/
>> -static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
>> +static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
>> {
>> - AVCodecInternal *avci = avctx->internal;
>> - AVPacket *const pkt = avci->in_pkt;
>> - const FFCodec *const codec = ffcodec(avctx->codec);
>> - int got_frame, actual_got_frame;
>> - int ret;
>> -
>> - if (!pkt->data && !avci->draining) {
>> - av_packet_unref(pkt);
>> - ret = ff_decode_get_packet(avctx, pkt);
>> - if (ret < 0 && ret != AVERROR_EOF)
>> - return ret;
>> - }
>> -
>> - // Some codecs (at least wma lossless) will crash when feeding drain packets
>> - // after EOF was signaled.
>> - if (avci->draining_done)
>> - return AVERROR_EOF;
>> -
>> - if (!pkt->data &&
>> - !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
>> - avctx->active_thread_type & FF_THREAD_FRAME))
>> - return AVERROR_EOF;
>> -
>> - got_frame = 0;
>> -
>> - if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
>> - ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
>> - } else {
>> - ret = codec->cb.decode(avctx, frame, &got_frame, pkt);
>> -
>> - if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
>> - frame->pkt_dts = pkt->dts;
>> - if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>> -#if FF_API_FRAME_PKT
>> -FF_DISABLE_DEPRECATION_WARNINGS
>> - if(!avctx->has_b_frames)
>> - frame->pkt_pos = pkt->pos;
>> -FF_ENABLE_DEPRECATION_WARNINGS
>> -#endif
>> - //FIXME these should be under if(!avctx->has_b_frames)
>> - /* get_buffer is supposed to set frame parameters */
>> - if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
>> - if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
>> - if (!frame->width) frame->width = avctx->width;
>> - if (!frame->height) frame->height = avctx->height;
>> - if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
>> - }
>> - }
>> - }
>> - emms_c();
>> - actual_got_frame = got_frame;
>> + AVCodecInternal *avci = avctx->internal;
>> + int ret = 0;
>>
>> if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>> if (frame->flags & AV_FRAME_FLAG_DISCARD)
>> - got_frame = 0;
>> + ret = AVERROR(EAGAIN);
>> } else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
>> uint8_t *side;
>> size_t side_size;
>> @@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> uint8_t skip_reason = 0;
>> uint8_t discard_reason = 0;
>>
>> - if (ret >= 0 && got_frame) {
>> if (frame->format == AV_SAMPLE_FMT_NONE)
>> frame->format = avctx->sample_fmt;
>> - if (!frame->ch_layout.nb_channels) {
>> - int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
>> - if (ret2 < 0) {
>> - ret = ret2;
>> - got_frame = 0;
>> - }
>> - }
>> + if (!frame->ch_layout.nb_channels)
>> + ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
>> #if FF_API_OLD_CHANNEL_LAYOUT
>> FF_DISABLE_DEPRECATION_WARNINGS
>> if (!frame->channel_layout)
>> @@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> #endif
>> if (!frame->sample_rate)
>> frame->sample_rate = avctx->sample_rate;
>> - }
>>
>> side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
>> if(side && side_size>=10) {
>> @@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> discard_reason = AV_RL8(side + 9);
>> }
>>
>> - if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame &&
>> + if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret &&
>> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>> avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples);
>> - got_frame = 0;
>> *discarded_samples += frame->nb_samples;
>> + ret = AVERROR(EAGAIN);
>> }
>>
>> - if (avci->skip_samples > 0 && got_frame &&
>> + if (avci->skip_samples > 0 && !ret &&
>> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>> if(frame->nb_samples <= avci->skip_samples){
>> - got_frame = 0;
>> *discarded_samples += frame->nb_samples;
>> avci->skip_samples -= frame->nb_samples;
>> av_log(avctx, AV_LOG_DEBUG, "skip whole frame, skip left: %d\n",
>> avci->skip_samples);
>> + ret = AVERROR(EAGAIN);
>> } else {
>> av_samples_copy(frame->extended_data, frame->extended_data, 0, avci->skip_samples,
>> frame->nb_samples - avci->skip_samples, avctx->ch_layout.nb_channels, frame->format);
>> @@ -432,11 +370,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> }
>> }
>>
>> - if (discard_padding > 0 && discard_padding <= frame->nb_samples && got_frame &&
>> + if (discard_padding > 0 && discard_padding <= frame->nb_samples && !ret &&
>> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>> if (discard_padding == frame->nb_samples) {
>> *discarded_samples += frame->nb_samples;
>> - got_frame = 0;
>> + ret = AVERROR(EAGAIN);
>> } else {
>> if(avctx->pkt_timebase.num && avctx->sample_rate) {
>> int64_t diff_ts = av_rescale_q(frame->nb_samples - discard_padding,
>> @@ -452,7 +390,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> }
>> }
>>
>> - if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && got_frame) {
>> + if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && !ret) {
>> AVFrameSideData *fside = av_frame_new_side_data(frame, AV_FRAME_DATA_SKIP_SAMPLES, 10);
>> if (fside) {
>> AV_WL32(fside->data, avci->skip_samples);
>> @@ -464,15 +402,88 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> }
>> }
>>
>> - if (!got_frame)
>> + return ret;
>> +}
>> +
>> +/*
>> + * The core of the receive_frame_wrapper for the decoders implementing
>> + * the simple API. Certain decoders might consume partial packets without
>> + * returning any output, so this function needs to be called in a loop until it
>> + * returns EAGAIN.
>> + **/
>> +static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
>> +{
>> + AVCodecInternal *avci = avctx->internal;
>> + AVPacket *const pkt = avci->in_pkt;
>> + const FFCodec *const codec = ffcodec(avctx->codec);
>> + int got_frame;
>> + int consumed, ret;
>> +
>> + if (!pkt->data && !avci->draining) {
>> + av_packet_unref(pkt);
>> + ret = ff_decode_get_packet(avctx, pkt);
>> + if (ret < 0 && ret != AVERROR_EOF)
>> + return ret;
>> + }
>> +
>> + // Some codecs (at least wma lossless) will crash when feeding drain packets
>> + // after EOF was signaled.
>> + if (avci->draining_done)
>> + return AVERROR_EOF;
>> +
>> + if (!pkt->data &&
>> + !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
>> + avctx->active_thread_type & FF_THREAD_FRAME))
>> + return AVERROR_EOF;
>> +
>> + got_frame = 0;
>> +
>> + if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
>> + consumed = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
>> + } else {
>> + consumed = codec->cb.decode(avctx, frame, &got_frame, pkt);
>> +
>> + if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
>> + frame->pkt_dts = pkt->dts;
>> + if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>> +#if FF_API_FRAME_PKT
>> +FF_DISABLE_DEPRECATION_WARNINGS
>> + if(!avctx->has_b_frames)
>> + frame->pkt_pos = pkt->pos;
>> +FF_ENABLE_DEPRECATION_WARNINGS
>> +#endif
>> + //FIXME these should be under if(!avctx->has_b_frames)
>> + /* get_buffer is supposed to set frame parameters */
>> + if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
>> + if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
>> + if (!frame->width) frame->width = avctx->width;
>> + if (!frame->height) frame->height = avctx->height;
>> + if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
>> + }
>> + }
>> + }
>> + emms_c();
>> +
>> + if (got_frame)
>> + ret = discard_samples(avctx, frame, discarded_samples);
>> + else
>> + ret = AVERROR(EAGAIN);
>> +
>> + if (consumed < 0)
>> + ret = consumed;
>> + if (consumed >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
>> + consumed = pkt->size;
>> +
>> + if (!got_frame || ret == AVERROR(EAGAIN))
>> av_frame_unref(frame);
>>
>> - if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
>> - ret = pkt->size;
>> + if (!ret)
>> + av_assert0(frame->buf[0]);
>> + if (ret == AVERROR(EAGAIN))
>> + ret = 0;
>>
>> - /* do not stop draining when actual_got_frame != 0 or ret < 0 */
>> - /* got_frame == 0 but actual_got_frame != 0 when frame is discarded */
>> - if (avci->draining && !actual_got_frame) {
>> + /* do not stop draining when got_frame != 0 or ret < 0 */
>> + if (avci->draining && !got_frame) {
>> if (ret < 0) {
>> /* prevent infinite loop if a decoder wrongly always return error on draining */
>> /* reasonable nb_errors_max = maximum b frames + thread count */
>> @@ -490,11 +501,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> }
>> }
>>
>> - if (ret >= pkt->size || ret < 0) {
>> + if (consumed >= pkt->size || ret < 0) {
>> av_packet_unref(pkt);
>> } else {
>> - int consumed = ret;
>> -
>> pkt->data += consumed;
>> pkt->size -= consumed;
>> pkt->pts = AV_NOPTS_VALUE;
>> @@ -509,10 +518,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> }
>> }
>>
>> - if (got_frame)
>> - av_assert0(frame->buf[0]);
>> -
>> - return ret < 0 ? ret : 0;
>> + return ret;
>> }
>>
>> #if CONFIG_LCMS2
>
> Would using --diff-algorithm=minimal (or patience or histogram) produce
> a more readable diff (i.e. one where the code that ends up in
> discard_samples() is moved instead of everything except it)?
No, "git format-patch --diff-algorithm={minimal,patience,histogram}"
produces the same output.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function
2023-07-08 19:00 [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function James Almer
` (2 preceding siblings ...)
2023-07-08 19:38 ` [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function Andreas Rheinhardt
@ 2023-07-09 9:48 ` Anton Khirnov
2023-07-09 12:11 ` James Almer
3 siblings, 1 reply; 11+ messages in thread
From: Anton Khirnov @ 2023-07-09 9:48 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting James Almer (2023-07-08 21:00:36)
> -/*
> - * The core of the receive_frame_wrapper for the decoders implementing
> - * the simple API. Certain decoders might consume partial packets without
> - * returning any output, so this function needs to be called in a loop until it
> - * returns EAGAIN.
> - **/
> -static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> +static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> {
> - AVCodecInternal *avci = avctx->internal;
> - AVPacket *const pkt = avci->in_pkt;
> - const FFCodec *const codec = ffcodec(avctx->codec);
> - int got_frame, actual_got_frame;
> - int ret;
> -
> - if (!pkt->data && !avci->draining) {
> - av_packet_unref(pkt);
> - ret = ff_decode_get_packet(avctx, pkt);
> - if (ret < 0 && ret != AVERROR_EOF)
> - return ret;
> - }
> -
> - // Some codecs (at least wma lossless) will crash when feeding drain packets
> - // after EOF was signaled.
> - if (avci->draining_done)
> - return AVERROR_EOF;
> -
> - if (!pkt->data &&
> - !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
> - avctx->active_thread_type & FF_THREAD_FRAME))
> - return AVERROR_EOF;
> -
> - got_frame = 0;
> -
> - if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
> - ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
> - } else {
> - ret = codec->cb.decode(avctx, frame, &got_frame, pkt);
> -
> - if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
> - frame->pkt_dts = pkt->dts;
> - if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> -#if FF_API_FRAME_PKT
> -FF_DISABLE_DEPRECATION_WARNINGS
> - if(!avctx->has_b_frames)
> - frame->pkt_pos = pkt->pos;
> -FF_ENABLE_DEPRECATION_WARNINGS
> -#endif
> - //FIXME these should be under if(!avctx->has_b_frames)
> - /* get_buffer is supposed to set frame parameters */
> - if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
> - if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
> - if (!frame->width) frame->width = avctx->width;
> - if (!frame->height) frame->height = avctx->height;
> - if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
> - }
> - }
> - }
> - emms_c();
> - actual_got_frame = got_frame;
> + AVCodecInternal *avci = avctx->internal;
> + int ret = 0;
>
> if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> if (frame->flags & AV_FRAME_FLAG_DISCARD)
> - got_frame = 0;
> + ret = AVERROR(EAGAIN);
It's quite strange an unexpected for a function called discard_samples()
to do anything with video. By leaving video processing in the caller you
also save a level of indentation.
> } else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
> uint8_t *side;
> size_t side_size;
> @@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
> uint8_t skip_reason = 0;
> uint8_t discard_reason = 0;
>
> - if (ret >= 0 && got_frame) {
> if (frame->format == AV_SAMPLE_FMT_NONE)
> frame->format = avctx->sample_fmt;
> - if (!frame->ch_layout.nb_channels) {
> - int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
> - if (ret2 < 0) {
> - ret = ret2;
> - got_frame = 0;
> - }
> - }
> + if (!frame->ch_layout.nb_channels)
> + ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
> #if FF_API_OLD_CHANNEL_LAYOUT
> FF_DISABLE_DEPRECATION_WARNINGS
> if (!frame->channel_layout)
> @@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
> #endif
> if (!frame->sample_rate)
> frame->sample_rate = avctx->sample_rate;
> - }
So the rest of the code below was previously executed even when there is
no frame? The change seems correct, but not I wouldn't expect it in a
'move' commit, so perhaps mention it in the commit message.
>
> side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
> if(side && side_size>=10) {
> @@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
> discard_reason = AV_RL8(side + 9);
> }
>
> - if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame &&
> + if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret &&
> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
> avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples);
> - got_frame = 0;
> *discarded_samples += frame->nb_samples;
> + ret = AVERROR(EAGAIN);
If I'm reading correctly, all the blocks below are conditioned on !ret,
so you might as well return here and drop those checks.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for all decoders
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for " James Almer
@ 2023-07-09 9:49 ` Anton Khirnov
2023-07-09 12:05 ` James Almer
0 siblings, 1 reply; 11+ messages in thread
From: Anton Khirnov @ 2023-07-09 9:49 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting James Almer (2023-07-08 21:00:38)
> And not just those with the old decode() API.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/decode.c | 52 +++++++++++++++++++++++----------------------
> 1 file changed, 27 insertions(+), 25 deletions(-)
Is there a functional reason for this patch?
I've been wondering recently about dropping this code entirely, because
get_buffer() should always fill those fields.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for all decoders
2023-07-09 9:49 ` Anton Khirnov
@ 2023-07-09 12:05 ` James Almer
2023-07-09 12:58 ` Anton Khirnov
0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2023-07-09 12:05 UTC (permalink / raw)
To: ffmpeg-devel
On 7/9/2023 6:49 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-08 21:00:38)
>> And not just those with the old decode() API.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/decode.c | 52 +++++++++++++++++++++++----------------------
>> 1 file changed, 27 insertions(+), 25 deletions(-)
>
> Is there a functional reason for this patch?
>
> I've been wondering recently about dropping this code entirely, because
> get_buffer() should always fill those fields.
Decoders that don't use get_buffer at all exist. If they don't fill all
frame fields, this would.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function
2023-07-09 9:48 ` Anton Khirnov
@ 2023-07-09 12:11 ` James Almer
2023-07-09 12:15 ` Anton Khirnov
0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2023-07-09 12:11 UTC (permalink / raw)
To: ffmpeg-devel
On 7/9/2023 6:48 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-08 21:00:36)
>> -/*
>> - * The core of the receive_frame_wrapper for the decoders implementing
>> - * the simple API. Certain decoders might consume partial packets without
>> - * returning any output, so this function needs to be called in a loop until it
>> - * returns EAGAIN.
>> - **/
>> -static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
>> +static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
>> {
>> - AVCodecInternal *avci = avctx->internal;
>> - AVPacket *const pkt = avci->in_pkt;
>> - const FFCodec *const codec = ffcodec(avctx->codec);
>> - int got_frame, actual_got_frame;
>> - int ret;
>> -
>> - if (!pkt->data && !avci->draining) {
>> - av_packet_unref(pkt);
>> - ret = ff_decode_get_packet(avctx, pkt);
>> - if (ret < 0 && ret != AVERROR_EOF)
>> - return ret;
>> - }
>> -
>> - // Some codecs (at least wma lossless) will crash when feeding drain packets
>> - // after EOF was signaled.
>> - if (avci->draining_done)
>> - return AVERROR_EOF;
>> -
>> - if (!pkt->data &&
>> - !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
>> - avctx->active_thread_type & FF_THREAD_FRAME))
>> - return AVERROR_EOF;
>> -
>> - got_frame = 0;
>> -
>> - if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
>> - ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
>> - } else {
>> - ret = codec->cb.decode(avctx, frame, &got_frame, pkt);
>> -
>> - if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
>> - frame->pkt_dts = pkt->dts;
>> - if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>> -#if FF_API_FRAME_PKT
>> -FF_DISABLE_DEPRECATION_WARNINGS
>> - if(!avctx->has_b_frames)
>> - frame->pkt_pos = pkt->pos;
>> -FF_ENABLE_DEPRECATION_WARNINGS
>> -#endif
>> - //FIXME these should be under if(!avctx->has_b_frames)
>> - /* get_buffer is supposed to set frame parameters */
>> - if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
>> - if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
>> - if (!frame->width) frame->width = avctx->width;
>> - if (!frame->height) frame->height = avctx->height;
>> - if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt;
>> - }
>> - }
>> - }
>> - emms_c();
>> - actual_got_frame = got_frame;
>> + AVCodecInternal *avci = avctx->internal;
>> + int ret = 0;
>>
>> if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>> if (frame->flags & AV_FRAME_FLAG_DISCARD)
>> - got_frame = 0;
>> + ret = AVERROR(EAGAIN);
>
> It's quite strange an unexpected for a function called discard_samples()
> to do anything with video. By leaving video processing in the caller you
> also save a level of indentation.
I could call it discard_samples_or_frame(), or discard_frame().
If i process the video in the caller, it will end up being duplicated in
patch 2, which goes against the point of factoring this out.
>
>> } else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
>> uint8_t *side;
>> size_t side_size;
>> @@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> uint8_t skip_reason = 0;
>> uint8_t discard_reason = 0;
>>
>> - if (ret >= 0 && got_frame) {
>> if (frame->format == AV_SAMPLE_FMT_NONE)
>> frame->format = avctx->sample_fmt;
>> - if (!frame->ch_layout.nb_channels) {
>> - int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
>> - if (ret2 < 0) {
>> - ret = ret2;
>> - got_frame = 0;
>> - }
>> - }
>> + if (!frame->ch_layout.nb_channels)
>> + ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
>> #if FF_API_OLD_CHANNEL_LAYOUT
>> FF_DISABLE_DEPRECATION_WARNINGS
>> if (!frame->channel_layout)
>> @@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> #endif
>> if (!frame->sample_rate)
>> frame->sample_rate = avctx->sample_rate;
>> - }
>
> So the rest of the code below was previously executed even when there is
> no frame? The change seems correct, but not I wouldn't expect it in a
> 'move' commit, so perhaps mention it in the commit message.
It still is, at least the block immediately after this, exporting
avci->skip_samples. I don't know how correct it is to do that, but i did
not want to change behavior in this set for decode() API decoders.
>
>>
>> side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
>> if(side && side_size>=10) {
>> @@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> discard_reason = AV_RL8(side + 9);
>> }
>>
>> - if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame &&
>> + if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret &&
>> !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>> avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples);
>> - got_frame = 0;
>> *discarded_samples += frame->nb_samples;
>> + ret = AVERROR(EAGAIN);
>
> If I'm reading correctly, all the blocks below are conditioned on !ret,
> so you might as well return here and drop those checks.
Ok.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function
2023-07-09 12:11 ` James Almer
@ 2023-07-09 12:15 ` Anton Khirnov
0 siblings, 0 replies; 11+ messages in thread
From: Anton Khirnov @ 2023-07-09 12:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting James Almer (2023-07-09 14:11:14)
>
> I could call it discard_samples_or_frame(), or discard_frame().
> If i process the video in the caller, it will end up being duplicated in
> patch 2, which goes against the point of factoring this out.
The duplicated code would be 2 very simple lines. The cost you're paying
for avoiding the duplication seems worse to me.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for all decoders
2023-07-09 12:05 ` James Almer
@ 2023-07-09 12:58 ` Anton Khirnov
0 siblings, 0 replies; 11+ messages in thread
From: Anton Khirnov @ 2023-07-09 12:58 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting James Almer (2023-07-09 14:05:45)
> On 7/9/2023 6:49 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-07-08 21:00:38)
> >> And not just those with the old decode() API.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >> libavcodec/decode.c | 52 +++++++++++++++++++++++----------------------
> >> 1 file changed, 27 insertions(+), 25 deletions(-)
> >
> > Is there a functional reason for this patch?
> >
> > I've been wondering recently about dropping this code entirely, because
> > get_buffer() should always fill those fields.
>
> Decoders that don't use get_buffer at all exist. If they don't fill all
> frame fields, this would.
Ah well, there's more of those than I thought.
No objections then.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-07-09 12:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-08 19:00 [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 2/3] avcodec/decode: check the output frame for discard samples with all decoders James Almer
2023-07-08 19:00 ` [FFmpeg-devel] [PATCH 3/3] avcodec/decode: fill missing frame fields for " James Almer
2023-07-09 9:49 ` Anton Khirnov
2023-07-09 12:05 ` James Almer
2023-07-09 12:58 ` Anton Khirnov
2023-07-08 19:38 ` [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function Andreas Rheinhardt
2023-07-08 19:41 ` James Almer
2023-07-09 9:48 ` Anton Khirnov
2023-07-09 12:11 ` James Almer
2023-07-09 12:15 ` Anton Khirnov
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