On Thu, 15 Oct 2015 02:26:46 +0200
Luca Barbato <[email protected]> wrote:
> Make it return an error and check its return value when it is used.
> Simplify the usage by calling `av_packet_ref` internally when needed.
> ---
> libavformat/utils.c | 64
> +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 78b5f59..06cba56 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -209,12 +209,23 @@ static int init_input(AVFormatContext *s, const char
> *filename,
> s, 0, s->probesize);
> }
>
> -static AVPacket *add_to_pktbuf(AVPacketList **packet_buffer, AVPacket *pkt,
> - AVPacketList **plast_pktl)
> +static int add_to_pktbuf(AVPacketList **packet_buffer, AVPacket *pkt,
> + AVPacketList **plast_pktl, int ref)
Not fond of bool parameters, bool parameters which change behavior in
such a fundamental way even though they look subtitle, and bool
parameters as 0/1 ints, instead of proper symbolic ints. But ok.
> {
> AVPacketList *pktl = av_mallocz(sizeof(AVPacketList));
> + int ret;
> +
> if (!pktl)
> - return NULL;
> + return AVERROR(ENOMEM);
> +
> + if (ref) {
> + if ((ret = av_packet_ref(&pktl->pkt, pkt)) < 0) {
> + av_free(pktl);
> + return ret;
> + }
> + } else {
> + pktl->pkt = *pkt;
> + }
>
> if (*packet_buffer)
> (*plast_pktl)->next = pktl;
> @@ -223,23 +234,21 @@ static AVPacket *add_to_pktbuf(AVPacketList
> **packet_buffer, AVPacket *pkt,
>
> /* Add the packet in the buffered packet list. */
> *plast_pktl = pktl;
> - pktl->pkt = *pkt;
> - return &pktl->pkt;
> + return 0;
> }
>
> static int queue_attached_pictures(AVFormatContext *s)
> {
> - int i;
> + int i, ret;
> for (i = 0; i < s->nb_streams; i++)
> if (s->streams[i]->disposition & AV_DISPOSITION_ATTACHED_PIC &&
> s->streams[i]->discard < AVDISCARD_ALL) {
> - AVPacket copy = s->streams[i]->attached_pic;
> - copy.buf = av_buffer_ref(copy.buf);
> - if (!copy.buf)
> - return AVERROR(ENOMEM);
>
> - add_to_pktbuf(&s->internal->raw_packet_buffer, ©,
> - &s->internal->raw_packet_buffer_end);
> + ret = add_to_pktbuf(&s->internal->raw_packet_buffer,
> + &s->streams[i]->attached_pic,
> + &s->internal->raw_packet_buffer_end, 1);
> + if (ret < 0)
> + return ret;
> }
> return 0;
> }
> @@ -443,8 +452,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
> !st->probe_packets))
> return ret;
>
> - add_to_pktbuf(&s->internal->raw_packet_buffer, pkt,
> - &s->internal->raw_packet_buffer_end);
> + err = add_to_pktbuf(&s->internal->raw_packet_buffer, pkt,
> + &s->internal->raw_packet_buffer_end, 0);
> + if (err)
> + return err;
> s->internal->raw_packet_buffer_remaining_size -= pkt->size;
>
> if ((err = probe_codec(s, st, pkt)) < 0)
> @@ -833,12 +844,10 @@ static int parse_packet(AVFormatContext *s, AVPacket
> *pkt, int stream_index)
> out_pkt.buf = pkt->buf;
> pkt->buf = NULL;
> }
> - if ((ret = av_dup_packet(&out_pkt)) < 0)
> - goto fail;
>
> - if (!add_to_pktbuf(&s->internal->parse_queue, &out_pkt,
> &s->internal->parse_queue_end)) {
> + if ((ret = add_to_pktbuf(&s->internal->parse_queue, &out_pkt,
> + &s->internal->parse_queue_end, 1) < 0)) {
> av_packet_unref(&out_pkt);
> - ret = AVERROR(ENOMEM);
> goto fail;
> }
> }
> @@ -1014,9 +1023,10 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
> return ret;
> }
>
> - if (av_dup_packet(add_to_pktbuf(&s->internal->packet_buffer, pkt,
> - &s->internal->packet_buffer_end)) <
> 0)
> - return AVERROR(ENOMEM);
> + ret = add_to_pktbuf(&s->internal->packet_buffer, pkt,
> + &s->internal->packet_buffer_end, 1);
> + if (ret < 0)
> + return ret;
> }
> }
>
> @@ -2173,12 +2183,12 @@ int avformat_find_stream_info(AVFormatContext *ic,
> AVDictionary **options)
> break;
> }
>
> - if (ic->flags & AVFMT_FLAG_NOBUFFER) {
> - pkt = &pkt1;
> - } else {
> - pkt = add_to_pktbuf(&ic->internal->packet_buffer, &pkt1,
> - &ic->internal->packet_buffer_end);
> - if ((ret = av_dup_packet(pkt)) < 0)
> + pkt = &pkt1;
> +
> + if (!(ic->flags & AVFMT_FLAG_NOBUFFER)) {
> + ret = add_to_pktbuf(&ic->internal->packet_buffer, &pkt1,
> + &ic->internal->packet_buffer_end, 1);
Maybe passing pkt instead of &pkt1 is slightly more readable.
> + if (ret < 0)
> goto find_stream_info_err;
> }
>
Otherwise ok... maybe.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
