[FFmpeg-devel] [PATCH 1/4] lavf: add write_uncoded_frame() API.

[Stefano Sabatini]

On date Wednesday 2014-01-01 13:27:05 +0100, Nicolas George encoded:
> 
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
>  libavformat/avformat.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/mux.c      |   43 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 6d719d7..9b25bff 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -453,6 +453,14 @@ typedef struct AVOutputFormat {
>  
>      void (*get_output_timestamp)(struct AVFormatContext *s, int stream,
>                                   int64_t *dts, int64_t *wall);
> +
> +    /**
> +     * Write an uncoded AVFrame

Missing .

> +     *
> +     * See av_write_uncoded_frame() for details.
> +     */
> +    int (*write_uncoded_frame)(struct AVFormatContext *, int stream_index,
> +                               AVFrame *frame, unsigned flags);
>  } AVOutputFormat;
>  /**
>   * @}
> @@ -1908,6 +1916,42 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt);
>  int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt);
>  
>  /**
> + * Write a uncoded frame to an output media file

Missing . at the end

> + *
> + * If the muxer supports it, this function allows to write an AVFrame
> + * structure directly, without encoding it into a packet.
> + * It is mostly useful for devices and similar special muxers that use raw
> + * video or PCM data and will not serialize it into a byte stream.
> + *

> + * To test whether it is possible to use it with a given muxer and stream,
> + * use the AV_WRITE_UNCODED_FRAME_QUERY flag with frame = NULL.

This sounds a bit weird. Indeed the feature should be directly
accessible, without the need to use some testing code. Also it is not
clear how the user is supposed to test the feature. Probably a small
test query function should be used instead.

> + *
> + * The caller keeps ownership of the frame and is responsible for freeing
> + * it.
> + *
> + * @return  >=0 for success, a negative code on error

nittt: return >= 0

> + */
> +int av_write_uncoded_frame(AVFormatContext *s, int stream_index,
> +                           AVFrame *frame, unsigned flags);

please shortly document the parameters, and/or refer to av_write_frame()

> +
> +enum AVWriteUncodedFrameFlags {
> +
> +    /**
> +     * Query whether the feature is possible on this stream.
> +     * The frame argument is ignored.
> +     */
> +    AV_WRITE_UNCODED_FRAME_QUERY           = 0x0001,
> +
> +    /**
> +     * Do not queue packets for interleaving.
> +     * If this flag is set, the application is responsible for interleaving.
> +     */
> +    AV_WRITE_UNCODED_FRAME_NO_INTERLEAVE   = 0x0002,
> +
> +};
> +
> +

Nittt: remove duplicate empty line

> +/**
>   * Write the stream trailer to an output media file and free the
>   * file private data.
>   *
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index f01b82b..0ea7dfc 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -414,6 +414,8 @@ int avformat_write_header(AVFormatContext *s, AVDictionary **options)
>      return 0;
>  }
>  
> +#define AV_PKT_FLAG_UNCODED_FRAME 0x2000
> +
>  //FIXME merge with compute_pkt_fields
>  static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt)
>  {
> @@ -536,7 +538,13 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
>      }
>  
>      did_split = av_packet_split_side_data(pkt);
> -    ret = s->oformat->write_packet(s, pkt);
> +    if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
> +        av_assert0(pkt->size == sizeof(AVFrame));
> +        ret = s->oformat->write_uncoded_frame(s, pkt->stream_index,
> +                                              (AVFrame *)pkt->data, 0);
> +    } else {
> +        ret = s->oformat->write_packet(s, pkt);
> +    }
>  
>      if (s->flush_packets && s->pb && ret >= 0 && s->flags & AVFMT_FLAG_FLUSH_PACKETS)
>          avio_flush(s->pb);
> @@ -871,3 +879,36 @@ int ff_write_chained(AVFormatContext *dst, int dst_stream, AVPacket *pkt,
>                                            dst->streams[dst_stream]->time_base);
>      return av_write_frame(dst, &local_pkt);
>  }
> +
> +int av_write_uncoded_frame(AVFormatContext *s, int stream_index,
> +                           AVFrame *frame, unsigned flags)
> +{
> +    AVPacket pkt, *pktp;
> +
> +    av_assert0(s->oformat);
> +    if (!s->oformat->write_uncoded_frame)
> +        return AVERROR(ENOSYS);
> +    if ((flags & AV_WRITE_UNCODED_FRAME_QUERY))
> +        return s->oformat->write_uncoded_frame(s, stream_index, NULL, flags);
> +
> +    if (!frame) {
> +        pktp = NULL;
> +    } else {
> +        pktp = &pkt;
> +        av_init_packet(&pkt);
> +        pkt.data = (void *)frame;

> +        /* Note: using sizeof(AVFrame) from outside lavu is unsafe in
> +           geneal, but it is only being used internally to this file as a

general

> +           consistency check. */
> +        pkt.size         = sizeof(*frame);

> +        pkt.pts          = frame->pts != AV_NOPTS_VALUE ? frame->pts : frame->pkt_pts;

probably unrelated, but I'm not sure this is correct since the codec
time base may be different from the stream time base

> +        pkt.dts          = frame->pkt_dts;
> +        pkt.duration     = av_frame_get_pkt_duration(frame);
> +        pkt.stream_index = stream_index;
> +        pkt.flags |= AV_PKT_FLAG_UNCODED_FRAME;
> +    }
> +
> +    return (flags & AV_WRITE_UNCODED_FRAME_NO_INTERLEAVE) ?
> +           av_write_frame(s, pktp) :
> +           av_interleaved_write_frame(s, pktp);
> +}

I'm not yet convinced the query flag is a good idea, but looks good to
me otherwise (but I'm not entitled to approve the patch, not in my
maintainership area).
-- 
FFmpeg = Fantastic and Friendly Merciless Peaceful Exciting Gigant