[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