| Message ID | HE1PR0301MB2154A19CAE4F9E2F0D89F8638F299@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,01/39] avcodec/audiotoolboxenc: Remove AV_CODEC_CAP_DR1 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 5/21/2021 6:17 AM, Andreas Rheinhardt wrote: > When the packet size is known in advance like here, one can avoid > an intermediate buffer for the packet data; also, there is no reason > to add AV_INPUT_BUFFER_MIN_SIZE to the packet size any more, as the > actually needed packet size can be easily calculated: It is three bytes > more than the raw nal size per NALU. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/libxavs.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/libxavs.c b/libavcodec/libxavs.c > index 253f4bde6a..ea53c49d38 100644 > --- a/libavcodec/libxavs.c > +++ b/libavcodec/libxavs.c > @@ -27,6 +27,7 @@ > #include <float.h> > #include <xavs.h> > #include "avcodec.h" > +#include "encode.h" > #include "internal.h" > #include "packet_internal.h" > #include "libavutil/internal.h" > @@ -85,18 +86,20 @@ static int encode_nals(AVCodecContext *ctx, AVPacket *pkt, > xavs_nal_t *nals, int nnal) > { > XavsContext *x4 = ctx->priv_data; > - uint8_t *p; > - int i, s, ret, size = x4->sei_size + AV_INPUT_BUFFER_MIN_SIZE; > + int64_t size = x4->sei_size; > + uint8_t *p, *p_end; > + int i, s, ret; > > if (!nnal) > return 0; > > for (i = 0; i < nnal; i++) > - size += nals[i].i_payload; > + size += 3U + nals[i].i_payload; > > - if ((ret = ff_alloc_packet2(ctx, pkt, size, 0)) < 0) > + if ((ret = ff_get_encode_buffer(ctx, pkt, size, 0)) < 0) > return ret; > p = pkt->data; > + p_end = pkt->data + size; > > /* Write the SEI as part of the first frame. */ > if (x4->sei_size > 0 && nnal > 0) { > @@ -106,12 +109,14 @@ static int encode_nals(AVCodecContext *ctx, AVPacket *pkt, > } > > for (i = 0; i < nnal; i++) { > + int size = p_end - p; > s = xavs_nal_encode(p, &size, 1, nals + i); > if (s < 0) > return -1; > + if (s != 3U + nals[i].i_payload) > + return AVERROR_BUG; AVERROR_EXTERNAL. This is not a bug in our code or something we can fix if it fails, so AVERROR_BUG is not correct (Neither would an assert for the same reason). > p += s; > } > - pkt->size = p - pkt->data; > > return 1; > } > @@ -150,7 +155,7 @@ static int XAVS_frame(AVCodecContext *avctx, AVPacket *pkt, > > if (!ret) { > if (!frame && !(x4->end_of_stream)) { > - if ((ret = ff_alloc_packet2(avctx, pkt, 4, 0)) < 0) > + if ((ret = ff_get_encode_buffer(avctx, pkt, 4, 0)) < 0) > return ret; > > pkt->data[0] = 0x0; > @@ -425,7 +430,8 @@ const AVCodec ff_libxavs_encoder = { > .init = XAVS_init, > .encode2 = XAVS_frame, > .close = XAVS_close, > - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS, > + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY | > + AV_CODEC_CAP_OTHER_THREADS, > .caps_internal = FF_CODEC_CAP_AUTO_THREADS, > .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE }, > .priv_class = &xavs_class, >
James Almer: > On 5/21/2021 6:17 AM, Andreas Rheinhardt wrote: >> When the packet size is known in advance like here, one can avoid >> an intermediate buffer for the packet data; also, there is no reason >> to add AV_INPUT_BUFFER_MIN_SIZE to the packet size any more, as the >> actually needed packet size can be easily calculated: It is three bytes >> more than the raw nal size per NALU. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavcodec/libxavs.c | 20 +++++++++++++------- >> 1 file changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/libavcodec/libxavs.c b/libavcodec/libxavs.c >> index 253f4bde6a..ea53c49d38 100644 >> --- a/libavcodec/libxavs.c >> +++ b/libavcodec/libxavs.c >> @@ -27,6 +27,7 @@ >> #include <float.h> >> #include <xavs.h> >> #include "avcodec.h" >> +#include "encode.h" >> #include "internal.h" >> #include "packet_internal.h" >> #include "libavutil/internal.h" >> @@ -85,18 +86,20 @@ static int encode_nals(AVCodecContext *ctx, >> AVPacket *pkt, >> xavs_nal_t *nals, int nnal) >> { >> XavsContext *x4 = ctx->priv_data; >> - uint8_t *p; >> - int i, s, ret, size = x4->sei_size + AV_INPUT_BUFFER_MIN_SIZE; >> + int64_t size = x4->sei_size; >> + uint8_t *p, *p_end; >> + int i, s, ret; >> if (!nnal) >> return 0; >> for (i = 0; i < nnal; i++) >> - size += nals[i].i_payload; >> + size += 3U + nals[i].i_payload; >> - if ((ret = ff_alloc_packet2(ctx, pkt, size, 0)) < 0) >> + if ((ret = ff_get_encode_buffer(ctx, pkt, size, 0)) < 0) >> return ret; >> p = pkt->data; >> + p_end = pkt->data + size; >> /* Write the SEI as part of the first frame. */ >> if (x4->sei_size > 0 && nnal > 0) { >> @@ -106,12 +109,14 @@ static int encode_nals(AVCodecContext *ctx, >> AVPacket *pkt, >> } >> for (i = 0; i < nnal; i++) { >> + int size = p_end - p; >> s = xavs_nal_encode(p, &size, 1, nals + i); >> if (s < 0) >> return -1; >> + if (s != 3U + nals[i].i_payload) >> + return AVERROR_BUG; > > AVERROR_EXTERNAL. This is not a bug in our code or something we can fix > if it fails, so AVERROR_BUG is not correct (Neither would an assert for > the same reason). > xavs's documentation is quite sparse, to say the least. It only says this (see https://github.com/pandastream/xavs/blob/master/xavs.h#L432): /* xavs_nal_encode: * encode a nal into a buffer, setting the size. * if b_annexeb then a long synch work is added * XXX: it currently doesn't check for overflow */ int xavs_nal_encode (void *, int *, int b_annexeb, xavs_nal_t * nal); So I chose AVERROR_BUG, because if this fails, then it might be due to me misunderstanding the xavs API. Obviously I don't believe that this will happen: It seems that development of xavs stopped years ago. >> p += s; >> } >> - pkt->size = p - pkt->data; >> return 1; >> } >> @@ -150,7 +155,7 @@ static int XAVS_frame(AVCodecContext *avctx, >> AVPacket *pkt, >> if (!ret) { >> if (!frame && !(x4->end_of_stream)) { >> - if ((ret = ff_alloc_packet2(avctx, pkt, 4, 0)) < 0) >> + if ((ret = ff_get_encode_buffer(avctx, pkt, 4, 0)) < 0) >> return ret; >> pkt->data[0] = 0x0; >> @@ -425,7 +430,8 @@ const AVCodec ff_libxavs_encoder = { >> .init = XAVS_init, >> .encode2 = XAVS_frame, >> .close = XAVS_close, >> - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS, >> + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY | >> + AV_CODEC_CAP_OTHER_THREADS, >> .caps_internal = FF_CODEC_CAP_AUTO_THREADS, >> .pix_fmts = (const enum AVPixelFormat[]) { >> AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE }, >> .priv_class = &xavs_class, >> > > _______________________________________________ > 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".
diff --git a/libavcodec/libxavs.c b/libavcodec/libxavs.c index 253f4bde6a..ea53c49d38 100644 --- a/libavcodec/libxavs.c +++ b/libavcodec/libxavs.c @@ -27,6 +27,7 @@ #include <float.h> #include <xavs.h> #include "avcodec.h" +#include "encode.h" #include "internal.h" #include "packet_internal.h" #include "libavutil/internal.h" @@ -85,18 +86,20 @@ static int encode_nals(AVCodecContext *ctx, AVPacket *pkt, xavs_nal_t *nals, int nnal) { XavsContext *x4 = ctx->priv_data; - uint8_t *p; - int i, s, ret, size = x4->sei_size + AV_INPUT_BUFFER_MIN_SIZE; + int64_t size = x4->sei_size; + uint8_t *p, *p_end; + int i, s, ret; if (!nnal) return 0; for (i = 0; i < nnal; i++) - size += nals[i].i_payload; + size += 3U + nals[i].i_payload; - if ((ret = ff_alloc_packet2(ctx, pkt, size, 0)) < 0) + if ((ret = ff_get_encode_buffer(ctx, pkt, size, 0)) < 0) return ret; p = pkt->data; + p_end = pkt->data + size; /* Write the SEI as part of the first frame. */ if (x4->sei_size > 0 && nnal > 0) { @@ -106,12 +109,14 @@ static int encode_nals(AVCodecContext *ctx, AVPacket *pkt, } for (i = 0; i < nnal; i++) { + int size = p_end - p; s = xavs_nal_encode(p, &size, 1, nals + i); if (s < 0) return -1; + if (s != 3U + nals[i].i_payload) + return AVERROR_BUG; p += s; } - pkt->size = p - pkt->data; return 1; } @@ -150,7 +155,7 @@ static int XAVS_frame(AVCodecContext *avctx, AVPacket *pkt, if (!ret) { if (!frame && !(x4->end_of_stream)) { - if ((ret = ff_alloc_packet2(avctx, pkt, 4, 0)) < 0) + if ((ret = ff_get_encode_buffer(avctx, pkt, 4, 0)) < 0) return ret; pkt->data[0] = 0x0; @@ -425,7 +430,8 @@ const AVCodec ff_libxavs_encoder = { .init = XAVS_init, .encode2 = XAVS_frame, .close = XAVS_close, - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS, + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY | + AV_CODEC_CAP_OTHER_THREADS, .caps_internal = FF_CODEC_CAP_AUTO_THREADS, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE }, .priv_class = &xavs_class,
When the packet size is known in advance like here, one can avoid an intermediate buffer for the packet data; also, there is no reason to add AV_INPUT_BUFFER_MIN_SIZE to the packet size any more, as the actually needed packet size can be easily calculated: It is three bytes more than the raw nal size per NALU. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/libxavs.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)