[FFmpeg-devel] [PATCH 08/10] avformat/avformat: Clarify documentation of av_interleaved_write_frame()

[Marton Balint]

On Tue, 31 Mar 2020, Andreas Rheinhardt wrote:

> The earlier documentation claimed that av_interleaved_write_frame()
> always orders by dts, which is not necessarily true when using muxers
> with custom interleavement functions or the audio_preload option.
>
> Furthermore, the documentation stated that libavformat takes ownership
> of the reference of the provided packet (if it is refcounted) and that
> the caller may not access the data through this reference after the
> function returns. This suggests that the returned packet is not blank,
> but instead still contains some set, but invalid fields, which implies
> that it would be dangerous to unreference this packet again.
>
> But this is not true: av_interleaved_write_frame()'s actual behaviour
> is to always output blank packet (even on error). This commit documents
> this fact so that callers know that they can directly reuse this packet.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> doc/APIchanges         |  4 ++++
> libavformat/avformat.h | 15 +++++++--------
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index f1d7eac2ee..31dc6c6c16 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,10 @@ libavutil:     2017-10-21
> 
> API changes, most recent first:
> 
> +2020-03-31 - xxxxxxxxxx - lavf 58.42.100 - avformat.h
> +  av_interleaved_write_frame() now guarantees to always return
> +  blank packets, even on failure.

Bump at least micro version if you add a changelog entry.

> +
> 2020-03-29 - xxxxxxxxxx - lavf 58.42.100 - avformat.h
>   av_read_frame() now guarantees to handle uninitialized input packets
>   and to return refcounted packets on success.
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 8f7466931a..9669ded1cd 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2596,7 +2596,7 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt);
>  * Write a packet to an output media file ensuring correct interleaving.
>  *
>  * This function will buffer the packets internally as needed to make sure the
> - * packets in the output file are properly interleaved in the order of
> + * packets in the output file are properly interleaved, usually ordered by
>  * increasing dts. Callers doing their own interleaving should call
>  * av_write_frame() instead of this function.
>  *
> @@ -2609,10 +2609,10 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt);
>  *            <br>
>  *            If the packet is reference-counted, this function will take
>  *            ownership of this reference and unreference it later when it sees
> - *            fit.
> - *            The caller must not access the data through this reference after
> - *            this function returns. If the packet is not reference-counted,
> - *            libavformat will make a copy.
> + *            fit. If the packet is not reference-counted, libavformat will
> + *            make a copy.
> + *            The returned packet will be blank (as if returned from
> + *            av_packet_alloc()), even on error.
>  *            <br>
>  *            This parameter can be NULL (at any time, not just at the end), to
>  *            flush the interleaving queues.
> @@ -2628,10 +2628,9 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt);
>  *            The dts for subsequent packets in one stream must be strictly
>  *            increasing (unless the output format is flagged with the
>  *            AVFMT_TS_NONSTRICT, then they merely have to be nondecreasing).
> - *            @ref AVPacket.duration "duration") should also be set if known.
> + *            @ref AVPacket.duration "duration" should also be set if known.
>  *
> - * @return 0 on success, a negative AVERROR on error. Libavformat will always
> - *         take care of freeing the packet, even if this function fails.
> + * @return 0 on success, a negative AVERROR on error.
>  *
>  * @see av_write_frame(), AVFormatContext.max_interleave_delta

Otherwise LGTM.

Thanks,
Marton