Marton Balint:
>
>
> On Sat, 11 Apr 2020, Andreas Rheinhardt wrote:
>
>> The documentation of av_write_frame() explicitly states that the function
>> doesn't take ownership of the packets sent to it; while av_write_frame()
>> does not directly unreference the packets after having written them, it
>> nevertheless modifies the packet in various ways:
>> 1. The timestamps might be modified either by prepare_input_packet() or
>> compute_muxer_pkt_fields().
>> 2. If a bitstream filter gets applied, it takes ownership of the
>> reference and the side-data in the packet sent to it.
>> In case of do_packet_auto_bsf(), the end result is that the returned
>> packet
>> contains the output of the last bsf in the chain. If an error happens,
>> a blank packet will be returned; a packet may also simply not lead to
>> any output (vp9_superframe).
>> This also implies that side data needs to be really copied and can't be
>> shared with the input packet.
>> The method choosen here minimizes copying of data: When the input isn't
>> refcounted and no bitstream filter is applied, the packet's data will
>> not be copied.
>>
>> Notice that packets that contain uncoded frames are exempt from this
>> because these packets are not owned by and returned to the user. This
>> also moves unreferencing the packets containing uncoded frames to
>> av_write_frame() in the noninterleaved codepath; in the interleaved
>> codepath, these packets are already freed in
>> av_interleaved_write_frame(),
>> so that unreferencing the packets in write_uncoded_frame_internal() is
>> no longer needed. It has been removed.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
>> ---
>> I was actually surprised when I realized how freeing uncoded frames in
>> the noninterleaved codepath could be left to av_write_frame().
>>
>> libavformat/mux.c | 48 +++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index cae9f42d11..48c0d4cd5f 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -874,11 +874,12 @@ static int do_packet_auto_bsf(AVFormatContext
>> *s, AVPacket *pkt) {
>> return 1;
>> }
>>
>> -int av_write_frame(AVFormatContext *s, AVPacket *pkt)
>> +int av_write_frame(AVFormatContext *s, AVPacket *in)
>> {
>> + AVPacket local_pkt, *pkt = &local_pkt;
>> int ret;
>>
>> - if (!pkt) {
>> + if (!in) {
>> if (s->oformat->flags & AVFMT_ALLOW_FLUSH) {
>> ret = s->oformat->write_packet(s, NULL);
>> flush_if_needed(s);
>> @@ -889,22 +890,49 @@ int av_write_frame(AVFormatContext *s, AVPacket
>> *pkt)
>> return 1;
>> }
>>
>> + if (in->flags & AV_PKT_FLAG_UNCODED_FRAME) {
>> + pkt = in;
>> + } else {
>> + /* We don't own in, so we have to make sure not to modify it.
>> + * The following avoids copying in's data unnecessarily.
>> + * Copying side data is unavoidable as a bitstream filter
>> + * may change it, e.g. free it on errors. */
>> + pkt->buf = NULL;
>> + pkt->data = in->data;
>> + pkt->size = in->size;
>> + ret = av_packet_copy_props(pkt, in);
>> + if (ret < 0)
>> + return ret;
>> + if (in->buf) {
>> + pkt->buf = av_buffer_ref(in->buf);
>> + if (!pkt->buf) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + }
>> + }
>> +
>> ret = prepare_input_packet(s, pkt);
>> if (ret < 0)
>> - return ret;
>> + goto fail;
>>
>> ret = do_packet_auto_bsf(s, pkt);
>> if (ret <= 0)
>> - return ret;
>> + goto fail;
>>
>> #if FF_API_COMPUTE_PKT_FIELDS2 && FF_API_LAVF_AVCTX
>> ret = compute_muxer_pkt_fields(s, s->streams[pkt->stream_index],
>> pkt);
>>
>> if (ret < 0 && !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
>> - return ret;
>> + goto fail;
>> #endif
>>
>> - return write_packet(s, pkt);
>> + ret = write_packet(s, pkt);
>> +
>> +fail:
>> + // Uncoded frames using the noninterleaved codepath are freed here
>
> This comment does not seem accurate. Pkt must always be unrefed here,
> not only for the uncoded_frames (where it happens to be == in), or I
> miss something? If not, then I'd just simply remove this comment.
Of course ordinary packets need to be unreferenced here, too; but I
thought that someone reading and potentially changing av_write_frame()
is already aware of that. But they might not be aware of the fact that
(contrary to the usual behaviour of av_write_frame()) some packets not
created in av_write_frame() are unreferenced there, hence this comment.
- Andreas
_______________________________________________
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".
