James Almer: > This generates a potential memory leak if info->pkt already contains side data > elements, and may mix side data from the last packet with other properties > from > the first.
Now that you mention it: Yes, there is a potential memleak.* So remove this. > > Keep all the properties from the first packet only in the output packet > instead. > > Signed-off-by: James Almer <jamr...@gmail.com> > --- > The alternative is to keep all the properties of the last packet instead of > the > first, or merge the side data from all packets into the output packet, giving > priority to the either the first or the last in case of duplicate side data > types. > > I have no idea what would be ideal since i don't have a sample that triggers > this code, and i don't know what kind of side data these eac3 packets could > contain that one would need to choose between them. > Right now any side data these packets might contain is ignored (with the potential exception of AVProducerReferenceTime stuff). Relevant side data would be AV_PKT_DATA_SKIP_SAMPLES (which the mov muxer ignores) or maybe AV_PKT_DATA_PARAM_CHANGE. > libavformat/movenc.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index bc8d08044e..bf3e4fa2ce 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -520,8 +520,6 @@ concatenate: > memcpy(info->pkt.data + info->pkt.size - pkt->size, pkt->data, > pkt->size); > info->num_blocks += num_blocks; > info->pkt.duration += pkt->duration; > - if ((ret = av_copy_packet_side_data(&info->pkt, pkt)) < 0) > - goto end; > if (info->num_blocks != 6) > goto end; > av_packet_unref(pkt); > - Andreas *: And anyway, av_copy_packet_side_data() is even more broken: It allocates a new side-data-array and only then checks for whether dst and src coincide. Was the intention behind this to enable a packet's side data to be made independently allocated in a scenario like AVPacket pkt1, pkt2; ... pkt1 = pkt2; av_copy_packet_side_data(&pkt1, &pkt1);? If so that is very dangerous given the av_packet_unref() of the destination packet in case of failure. (And if src != dst, then the already allocated side data leaks in case of allocation failure because dst->side_data_elems does not accurately reflect how many side data elements there are right now.) _______________________________________________ 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".
