1. When an error happened in ff_interleave_add_packet() during
interleaving a packet for output, said packet would not be unreferenced
in ff_interleave_add_packet(), but would be zeroed in
av_interleaved_write_frame(), which results in a memleak.
2. When no error happened in ff_interleave_add_packet() during
interleaving a packet for output, the input packet will already be blank
(as if av_packet_unref() had been applied to it), but it will
nevertheless be zeroed and initialized again. This is unnecessary.
Given the possibility of using other functions for interleavement than
ff_interleave_packet_per_dts(), this commit sets and documents what
interleavement functions need to do: On success, the input packet (if
given) should be a blank packet on return. On failure, cleaning up will
be done by av_interleave_write_frame() (where the already existing code
for error handling can be reused). ff_audio_rechunk_interleave() has been
changed to abide by this. In order to keep these requirements
consistent, they are kept in one place that is referenced by the other
documentations.
Updating the documentation incidentally also removed another reference
to AVPacket's destructor.
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
---
libavformat/audiointerleave.c | 1 +
libavformat/audiointerleave.h | 4 ++++
libavformat/avformat.h | 1 +
libavformat/internal.h | 16 +++++++---------
libavformat/mux.c | 30 +++++++++++-------------------
5 files changed, 24 insertions(+), 28 deletions(-)
diff --git a/libavformat/audiointerleave.c b/libavformat/audiointerleave.c
index f10c83d7c9..d3b35d804e 100644
--- a/libavformat/audiointerleave.c
+++ b/libavformat/audiointerleave.c
@@ -121,6 +121,7 @@ int ff_audio_rechunk_interleave(AVFormatContext *s,
AVPacket *out, AVPacket *pkt
aic->fifo_size = new_size;
}
av_fifo_generic_write(aic->fifo, pkt->data, pkt->size, NULL);
+ av_packet_unref(pkt);
} else {
// rewrite pts and dts to be decoded time line position
pkt->pts = pkt->dts = aic->dts;
diff --git a/libavformat/audiointerleave.h b/libavformat/audiointerleave.h
index 0933310f4c..d21647b6b1 100644
--- a/libavformat/audiointerleave.h
+++ b/libavformat/audiointerleave.h
@@ -48,6 +48,10 @@ void ff_audio_interleave_close(AVFormatContext *s);
*
* @param get_packet function will output a packet when streams are correctly
interleaved.
* @param compare_ts function will compare AVPackets and decide interleaving
order.
+ *
+ * Apart from these two functions, this function behaves like
ff_interleave_packet_per_dts.
+ *
+ * @note This function must not be used with uncoded audio frames.
*/
int ff_audio_rechunk_interleave(AVFormatContext *s, AVPacket *out, AVPacket
*pkt, int flush,
int (*get_packet)(AVFormatContext *, AVPacket *,
AVPacket *, int),
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 39b99b4481..8f7466931a 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -553,6 +553,7 @@ typedef struct AVOutputFormat {
/**
* A format-specific function for interleavement.
* If unset, packets will be interleaved by dts.
+ * See the description of ff_interleave_packet_per_dts for details.
*/
int (*interleave_packet)(struct AVFormatContext *, AVPacket *out,
AVPacket *in, int flush);
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 332477a532..a861acdc2a 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -230,9 +230,9 @@ char *ff_data_to_hex(char *buf, const uint8_t *src, int
size, int lowercase);
int ff_hex_to_data(uint8_t *data, const char *p);
/**
- * Add packet to AVFormatContext->packet_buffer list, determining its
+ * Add packet to an AVFormatContext's packet_buffer list, determining its
* interleaved position using compare() function argument.
- * @return 0, or < 0 on error
+ * @return 0, or < 0 on error. On success, pkt will be blank (as if
unreferenced).
*/
int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt,
int (*compare)(AVFormatContext *, const AVPacket
*, const AVPacket *));
@@ -495,15 +495,13 @@ int ff_framehash_write_header(AVFormatContext *s);
int ff_read_packet(AVFormatContext *s, AVPacket *pkt);
/**
- * Interleave a packet per dts in an output media file.
- *
- * Packets with pkt->destruct == av_destruct_packet will be freed inside this
- * function, so they cannot be used after it. Note that calling
av_packet_unref()
- * on them is still safe.
+ * Interleave an AVPacket per dts so it can be muxed.
*
- * @param s media file handle
+ * @param s an AVFormatContext for output. in resp. out will be added to
+ * resp. taken from its packet buffer.
* @param out the interleaved packet will be output here
- * @param pkt the input packet
+ * @param in the input packet. If not NULL will be a blank packet
+ * (as if unreferenced) on success.
* @param flush 1 if no further packets are available as input and all
* remaining packets should be output
* @return 1 if a packet was output, 0 if no packet could be output,
diff --git a/libavformat/mux.c b/libavformat/mux.c
index a0ebcec119..79731d3008 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1166,20 +1166,13 @@ int ff_interleaved_peek(AVFormatContext *s, int stream,
/**
* Interleave an AVPacket correctly so it can be muxed.
- * @param out the interleaved packet will be output here
- * @param in the input packet
- * @param flush 1 if no further packets are available as input and all
- * remaining packets should be output
- * @return 1 if a packet was output, 0 if no packet could be output,
- * < 0 if an error occurred
+ *
+ * See the description of ff_interleave_packet_per_dts for details.
*/
static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket *in,
int flush)
{
if (s->oformat->interleave_packet) {
- int ret = s->oformat->interleave_packet(s, out, in, flush);
- if (in)
- av_packet_unref(in);
- return ret;
+ return s->oformat->interleave_packet(s, out, in, flush);
} else
return ff_interleave_packet_per_dts(s, out, in, flush);
}
@@ -1222,14 +1215,13 @@ int av_interleaved_write_frame(AVFormatContext *s,
AVPacket *pkt)
for (;; ) {
AVPacket opkt;
- int ret = interleave_packet(s, &opkt, pkt, flush);
- if (pkt) {
- memset(pkt, 0, sizeof(*pkt));
- av_init_packet(pkt);
- pkt = NULL;
- }
- if (ret <= 0) //FIXME cleanup needed for ret<0 ?
- return ret;
+ ret = interleave_packet(s, &opkt, pkt, flush);
+ if (ret < 0)
+ goto fail;
+ if (!ret)
+ return 0;
+
+ pkt = NULL;
ret = write_packet(s, &opkt);
@@ -1242,7 +1234,7 @@ fail:
// This is a deviation from the usual behaviour
// of av_interleaved_write_frame: We leave cleaning
// up uncoded frames to write_uncoded_frame_internal.
- if (!(pkt->flags & AV_PKT_FLAG_UNCODED_FRAME))
+ if (pkt && !(pkt->flags & AV_PKT_FLAG_UNCODED_FRAME))
av_packet_unref(pkt);
return ret;
}
--
2.20.1
_______________________________________________
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".
