[FFmpeg-devel] [PATCH 6/8] avformat/mux: do not destroy packets of av_write_frame on bitstream filtering
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sat Mar 28 22:56:06 EET 2020
Marton Balint:
> av_write_frame() does not take ownership of the packet.
>
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
> libavformat/mux.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 3054ab8644..706fdcbbf4 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -935,7 +935,16 @@ static int write_packets_common(AVFormatContext *s, AVStream *st, AVPacket *pkt,
> if (ret < 0) {
> if (ret == AVERROR(EAGAIN) && !consumed_packet) {
> av_assert2(sti->bsfcs_idx == 0);
> - ret = av_bsf_send_packet(sti->bsfcs[0], pkt);
> + if (!interleaved && pkt) {
> + AVPacket tmp;
> + ret = av_packet_ref(&tmp, pkt);
> + if (ret < 0)
> + goto fail;
> + ret = av_bsf_send_packet(sti->bsfcs[0], &tmp);
> + av_packet_unref(&tmp);
> + } else {
> + ret = av_bsf_send_packet(sti->bsfcs[0], pkt);
> + }
> av_assert2(ret != AVERROR(EAGAIN));
> if (ret >= 0) {
> consumed_packet = 1;
>
When I proposed something similar in [1] (based upon exactly the same
thinking as you with your patch), I was told that the owner of a packet
just has the obligation to free it; and not the right to expect others
not to modify it. I changed my mind on this: Given that av_write_frame()
does not take a const AVPacket * as parameter, the caller has no right
to believe that the packets are returned untouched.
Furthermore, you are using an AVPacket on the stack, yet I thought that
this should be avoided, because sizeof(AVPacket) should eventually no
longer be part of the public API.
- Andreas
[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248153.html
More information about the ffmpeg-devel
mailing list