| Message ID | Nh4mqjI--3-9@lynne.ee |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] lavc/internal: add skip_samples2 field | 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 |
Correct patch attached.
Current interaction between AV_FRAME_DATA_SKIP_SAMPLES and AVCodecContext.skip_samples seems unncecessarily complicated to me and you're just making it worse. Is there any reason we can't drop AVCodecContext.skip_samples entirely and signal it purely through side data? Then decoders could fully control everything they wish by modifying side data on output frames.
Oct 19, 2023, 10:39 by anton@khirnov.net: > Current interaction between AV_FRAME_DATA_SKIP_SAMPLES and > AVCodecContext.skip_samples seems unncecessarily complicated to me and > you're just making it worse. > > Is there any reason we can't drop AVCodecContext.skip_samples entirely > and signal it purely through side data? Then decoders could fully > control everything they wish by modifying side data on output frames. > You mean let the decoder parse skip samples side data, strip it from the packet, and attach a new side data to the frame?
On 10/19/2023 5:39 AM, Anton Khirnov wrote: > Current interaction between AV_FRAME_DATA_SKIP_SAMPLES and > AVCodecContext.skip_samples seems unncecessarily complicated to me and > you're just making it worse. > > Is there any reason we can't drop AVCodecContext.skip_samples entirely > and signal it purely through side data? Then decoders could fully > control everything they wish by modifying side data on output frames. Given that now coded_side_data is fully implemented for decoding and encoding, including getting elements from the container, it's probably a good idea to do that.
Quoting Lynne (2023-10-19 14:49:46) > Oct 19, 2023, 10:39 by anton@khirnov.net: > > > Current interaction between AV_FRAME_DATA_SKIP_SAMPLES and > > AVCodecContext.skip_samples seems unncecessarily complicated to me and > > you're just making it worse. > > > > Is there any reason we can't drop AVCodecContext.skip_samples entirely > > and signal it purely through side data? Then decoders could fully > > control everything they wish by modifying side data on output frames. > > > > You mean let the decoder parse skip samples side data, > strip it from the packet, and attach a new side data to the frame? Not from the packet - that should be const for decoders. The generic code currently translates AV_PKT_DATA_SKIP_SAMPLES from the packet to the frame in ff_get_buffer(). The decoder can then override that in the frame.
From 9986c7f0c71d944101f1c7fe7b1395ee21e34a8e Mon Sep 17 00:00:00 2001 From: Lynne <dev@lynne.ee> Date: Thu, 19 Oct 2023 04:28:03 +0200 Subject: [PATCH 1/2] lavc/internal: add skip_samples2 field The issue is that avci->skip_samples will be overridden by any side-data. When operating on raw files (adts, for example), the decoder is free to decide the amount of samples to skip. Usually, this is the algorithmic delay of the decoder. When operating on more complete containers, like ISOBMFF, the amount of samples to be skipped is recorded and signalled by the encoder. However, it turns out many encoders have an arbitrary choice of padding to insert at the start. Normally, they would signal the amount into the container. But with ISOBMFF, there isn't just a single option - the format has been extended multiple times, and has multiple ways to signal padding. In the case of fdkaac-encoded samples, the STTS is used, rather than the CTTS, which ends up with us leaving the padding in. But it's not just containers, as it turns out, most AAC encoders use an arbitrary amount of padding at the start that may, or may not be trimmed (usually, it won't be). Furthermore, AAC has specific amount of algorithmic delay for SBR operation. This delay is not accounter for anywhere. While it's an option to skip the samples in the decoder, doing this in decode.c, along with the rest of the skip adjustments, is a neater way, and can be extended to other codecs. --- libavcodec/decode.c | 2 ++ libavcodec/internal.h | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index ad39021354..32944a6b6a 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -338,6 +338,8 @@ static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *disca return AVERROR(EAGAIN); } + avci->skip_samples += avci->skip_samples2; + if (avci->skip_samples > 0) { if (frame->nb_samples <= avci->skip_samples){ *discarded_samples += frame->nb_samples; diff --git a/libavcodec/internal.h b/libavcodec/internal.h index eb9e0d707c..3d8d4d9a4d 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -115,9 +115,18 @@ typedef struct AVCodecInternal { /** * Number of audio samples to skip at the start of the next decoded frame + * + * Note: This will be overridden by any side data. */ int skip_samples; + /** + * Additional samples to skip ad the start of the next decoded frame. + * + * These will be added to any skip amount after taking side data into account. + */ + int skip_samples2; + /** * hwaccel-specific private data */ -- 2.42.0