On 12/4/2020 4:08 PM, Andreas Rheinhardt wrote:
James Almer:
On 12/4/2020 2:08 PM, Andreas Rheinhardt wrote:
James Almer:
Signed-off-by: James Almer <jamr...@gmail.com>
---
The idea of making avpriv_packet_list_* public was not liked, and it was
suggested to just use AVFifoBuffer instead.
After this, the avpriv_packet_list_* functions can be made local to
libavformat again.
libavcodec/decode.c | 41 +++++++++++++++++++++++++++++------------
libavcodec/internal.h | 4 ++--
libavcodec/utils.c | 11 ++++++-----
3 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 5a1849f944..0550637029 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -145,22 +145,40 @@ fail2:
#define IS_EMPTY(pkt) (!(pkt)->data)
+static int copy_packet_props(void *_src, void *_dst, int size)
+{
+ AVPacket *src = _src, *dst = _dst;
+ int ret;
+
+ av_init_packet(dst);
+ ret = av_packet_copy_props(dst, src);
+ if (ret < 0)
+ return ret;> +
+ dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
+ dst->data = (void*)1; // HACK: Needed for IS_EMPTY().
+
+ return sizeof(*dst);
+}
This is not how a write function for a fifo should look like: A fifo may
need to store the beginning of a packet at the end of the fifo and the
end of a packet at the beginning of a fifo; you can check for this by
checking whether size is < sizeof(AVPacket).
I'm already ensuring there's always sizeof(AVPacket) bytes left before
calling av_fifo_generic_write().
And? This just means one can write sizeof(AVPacket) bytes to the fifo,
not that there are sizeof(AVPacket) contiguous bytes available at the
current write pointer. The free space might be partially at the end and
partially at the beginning of the fifo buffer.
It wraps around? This is not obvious from reading the doxy.
In any case, by always incrementing the FIFO by sizeof(AVPacket) bytes
it shouldn't be an issue since it will always be a multiple of it. But
as i said, i'll just do it all outside of av_fifo_generic_write() anyway
since i can't propagate errors.
(The current implementation seems to actually only allocate multiples of
a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
calls use only multiples of this unit, but it doesn't seem to be
documented. Should probably be done as this simplifies using fifo
arrays.)
Return values for av_fifo_generic_read are also undocumented. Currently,
it's always 0 no matter what.
And apart from that: The current implementation of
av_fifo_generic_write() does not forward errors from the write function;
and changing this require be an API break.
Accepting a function that can return < 0 but must not be an error sounds
like an awful oversight when defining this API...
I guess i can just do the av_packet_copy_props() call in
extract_packet_props() and not use a custom function at all.
+
static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
{
int ret = 0;
- ret = avpriv_packet_list_put(&avci->pkt_props,
&avci->pkt_props_tail, pkt,
- av_packet_copy_props, 0);
+ if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
+ ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = av_fifo_generic_write(avci->pkt_props, pkt, sizeof(*pkt),
copy_packet_props);
if (ret < 0)
return ret;
- avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for
ff_decode_frame_props().
- avci->pkt_props_tail->pkt.data = (void*)1; // HACK: Needed for
IS_EMPTY().
+ av_assert0(ret == sizeof(*pkt));
if (IS_EMPTY(avci->last_pkt_props)) {
- ret = avpriv_packet_list_get(&avci->pkt_props,
- &avci->pkt_props_tail,
- avci->last_pkt_props);
- av_assert0(ret != AVERROR(EAGAIN));
+ ret = av_fifo_generic_read(avci->pkt_props,
avci->last_pkt_props,
+ sizeof(*avci->last_pkt_props),
NULL);
}
return ret;
}
@@ -1721,10 +1739,9 @@ int ff_decode_frame_props(AVCodecContext
*avctx, AVFrame *frame)
{ AV_PKT_DATA_S12M_TIMECODE,
AV_FRAME_DATA_S12M_TIMECODE },
};
- if (IS_EMPTY(pkt))
- avpriv_packet_list_get(&avctx->internal->pkt_props,
- &avctx->internal->pkt_props_tail,
- pkt);
+ if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >=
sizeof(*pkt))
+ av_fifo_generic_read(avctx->internal->pkt_props,
+ pkt, sizeof(*pkt), NULL);
if (pkt) {
frame->pts = pkt->pts;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 17defb9b50..8a51bca2df 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -28,6 +28,7 @@
#include "libavutil/buffer.h"
#include "libavutil/channel_layout.h"
+#include "libavutil/fifo.h"
#include "libavutil/mathematics.h"
#include "libavutil/pixfmt.h"
#include "avcodec.h"
@@ -145,8 +146,7 @@ typedef struct AVCodecInternal {
* for decoding.
*/
AVPacket *last_pkt_props;
- AVPacketList *pkt_props;
- AVPacketList *pkt_props_tail;
+ AVFifoBuffer *pkt_props;
/**
* temporary buffer used for encoders to store their bitstream
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 45295dd3ce..c81cc972dc 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -593,10 +593,12 @@ int attribute_align_arg
avcodec_open2(AVCodecContext *avctx, const AVCodec *code
avci->es.in_frame = av_frame_alloc();
avci->ds.in_pkt = av_packet_alloc();
avci->last_pkt_props = av_packet_alloc();
+ avci->pkt_props = av_fifo_alloc(sizeof(AVPacket));
if (!avci->compat_decode_frame || !avci->compat_encode_packet ||
!avci->buffer_frame || !avci->buffer_pkt ||
!avci->es.in_frame || !avci->ds.in_pkt ||
- !avci->to_free || !avci->last_pkt_props) {
+ !avci->to_free || !avci->last_pkt_props ||
+ !avci->pkt_props) {
ret = AVERROR(ENOMEM);
goto free_and_end;
}
@@ -1076,6 +1078,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
av_packet_free(&avci->compat_encode_packet);
av_packet_free(&avci->buffer_pkt);
av_packet_free(&avci->last_pkt_props);
+ av_fifo_freep(&avci->pkt_props);
av_packet_free(&avci->ds.in_pkt);
av_frame_free(&avci->es.in_frame);
@@ -1116,8 +1119,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
av_packet_unref(avci->buffer_pkt);
av_packet_unref(avci->last_pkt_props);
- avpriv_packet_list_free(&avci->pkt_props,
- &avci->pkt_props_tail);
+ av_fifo_reset(avci->pkt_props);
The packets in the fifo may contain side-data (because
av_packet_copy_props() copies it) and if so, it leaks here.
Right. Will drain it, then.
av_frame_unref(avci->es.in_frame);
av_packet_unref(avci->ds.in_pkt);
@@ -1180,8 +1182,7 @@ av_cold int avcodec_close(AVCodecContext *avctx)
av_packet_free(&avctx->internal->compat_encode_packet);
av_packet_free(&avctx->internal->buffer_pkt);
av_packet_free(&avctx->internal->last_pkt_props);
- avpriv_packet_list_free(&avctx->internal->pkt_props,
- &avctx->internal->pkt_props_tail);
+ av_fifo_freep(&avctx->internal->pkt_props);
av_packet_free(&avctx->internal->ds.in_pkt);
av_frame_free(&avctx->internal->es.in_frame);
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
