[FFmpeg-devel] [PATCH 06/11] avformat: add av_abort_output() function

[Nicolas George]

Le sextidi 16 thermidor, an CCXXIV, sebechlebskyjan at gmail.com a écrit :
> From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> 
> Signed-off-by: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> ---
>  libavformat/avformat.h | 14 ++++++++++++++
>  libavformat/mux.c      | 16 ++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9191c69..9173908 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2510,6 +2510,8 @@ int av_write_uncoded_frame_query(AVFormatContext *s, int stream_index);
>   *
>   * If AVFMT_FLAG_NONBLOCK is set, this call may return AVERROR(EAGAIN)
>   * meaning the operation is pending and the call should be repeated.
> + * If caller decides to abort operation (after too many calls have returned
> + * AVERROR(EAGAIN)), it can be done by calling @ref av_abort_output().
>   *
>   * @param s media file handle
>   * @return 0 if OK, AVERROR(EAGAIN) in case call should be repeated,
> @@ -2518,6 +2520,18 @@ int av_write_uncoded_frame_query(AVFormatContext *s, int stream_index);
>  int av_write_trailer(AVFormatContext *s);
>  
>  /**
> + * Abort non-blocking muxer operation and free private data.
> + *
> + * May only be called after a successful call to avformat_write_header,
> + * and used only with muxer operating in non-blocking mode (AVFMT_FLAG_NONBLOCK)
> + * must be set.
> + *
> + * @param s media file handle
> + * return >= 0 on success, negative AVERROR on error
> + */

> +int av_abort_output(AVFormatContext *s);

The other functions are called av_write_something() or
avformat_write_something(): maybe avformat_write_abort()?

Also, it could call avformat_free_context() and set s to NULL, unless there
is some use in reusing the context?

> +
> +/**
>   * Return the output format in the list of registered output formats
>   * which best matches the provided parameters, or return NULL if
>   * there is no match.
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index bc9c98f..888a9f1 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -1267,6 +1267,22 @@ fail:
>      return ret;
>  }
>  
> +int av_abort_output(AVFormatContext *s)
> +{
> +    int ret;
> +

> +    if (!(s->flags & AVFMT_FLAG_NONBLOCK))
> +        return AVERROR(EINVAL);

Since the application should be able to know whether the muxer is in
blocking mode, it could be an assert.

But is it really necessary? Applications in blocking mode may want to exit
immediately too.

Anyway, any of these changes would allow to make the function void.

> +
> +    ret = av_write_trailer(s);
> +    if (ret == AVERROR(EAGAIN)) {

Is it useful? The application could try and write the trailer itself,
possibly allowing a little more time to finish. And only call
av_abort_output() when it really wants to stop now.

> +        deinit_muxer(s);

There is a subtle pitfall here: if the muxer does not have a deinit function
and av_write_trailer() returned EAGAIN (or was not called like I suggest),
then the resources leak.

Fortunately, we currently do not have any muxer that both supports
non-blocking mode and has a potentially blocking write_trailer(). We can
decide that any future muxer that would must have a deinit() function. In
that case, we can get away with the following steps:

  1. If the muxer has a deinit() method, just call deinit_muxer().

  2. Else, sabotage the context's AVIO stream so that it returns an error
     for all operations and call av_write_trailer().

The "sabotage" part of 2 is not very robust, but I think it is fine enough
for our use case. In fact, I would not object if that detail was left as a
TODO comment.

> +        ret = 0;
> +    }
> +
> +    return ret;
> +}
> +
>  int av_get_output_timestamp(struct AVFormatContext *s, int stream,
>                              int64_t *dts, int64_t *wall)
>  {

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160803/546b3feb/attachment.sig>