On Sat, 28 Mar 2020, Andreas Rheinhardt wrote:
Marton Balint:
av_write_frame() does not take ownership of the packet.
Signed-off-by: Marton Balint <c...@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.
pts/dts/duration is one thing. But if the user must free the data (or the
buffer) then the function must NOT. Is it a valid expectation from the
user to have the data available after the av_write_frame() call? I think
so.
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.
IMHO we should make reasonable effort to not change the behaviour of
the API, even if it is not explictly documented. Freeing data/buf passed
to av_write_frame() seems like a breaking change to me. If you overwrite
pkt->data to NULL in av_write_frame then fate-fifo-muxer-tst will fail.
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.
Is there still interest to do this? We have tons of static AVPacket
allocations all over the codebase because it is simple. Even in new code.
I don't think the added complexity actually worth removing it.
Regards,
Marton
_______________________________________________
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".
