[FFmpeg-devel] [PATCH v2 04/11] avformat/muxers: Add non-blocking mode support for av_write_trailer

[Michael Niedermayer]

On Thu, Sep 01, 2016 at 09:45:49PM +0200, Jan Sebechlebsky wrote:
> 
> 
> On 08/26/2016 12:59 AM, Jan Sebechlebsky wrote:
> >On 08/22/2016 04:49 PM, Michael Niedermayer wrote:
> >
> >>On Mon, Aug 22, 2016 at 04:27:16PM +0200, Jan Sebechlebsky wrote:
> >>>
> >>>On 08/22/2016 09:51 AM, Michael Niedermayer wrote:
> >>>>On Thu, Aug 11, 2016 at 02:38:29PM +0200,
> >>>>sebechlebskyjan at gmail.com wrote:
> >>>>>From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> >>>>>
> >>>>>This makes av_write_trailer not to free the resources if
> >>>>>write_trailer
> >>>>>call returns AVERROR(EAGAIN) allowing repeated calls of
> >>>>>write_trailer of
> >>>>>non-blocking muxer.
> >>>>>
> >>>>>Signed-off-by: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> >>>>>---
> >>>>>  Changes since the last version of the patch:
> >>>>>  - Added assert to the part of the code dealing with flushing
> >>>>>    interleaved packets which should not be entered if
> >>>>>    muxer in non-blocking mode is used.
> >>>>>    (also there is assert for the same condition added into
> >>>>>     av_interleaved_write_packet in one of the following
> >>>>>     patches).
> >>>>>
> >>>>>  libavformat/avformat.h |  6 +++++-
> >>>>>  libavformat/mux.c      | 10 ++++++++--
> >>>>>  2 files changed, 13 insertions(+), 3 deletions(-)
> >>>>>
> >>>>>diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >>>>>index d8a6cf3..2cc3156 100644
> >>>>>--- a/libavformat/avformat.h
> >>>>>+++ b/libavformat/avformat.h
> >>>>>@@ -2510,8 +2510,12 @@ int
> >>>>>av_write_uncoded_frame_query(AVFormatContext *s, int
> >>>>>stream_index);
> >>>>>   *
> >>>>>   * May only be called after a successful call to
> >>>>>avformat_write_header.
> >>>>>   *
> >>>>>+ * If AVFMT_FLAG_NONBLOCK is set, this call may return
> >>>>>AVERROR(EAGAIN)
> >>>>>+ * meaning the operation is pending and the call should be repeated.
> >>>>>+ *
> >>>>>   * @param s media file handle
> >>>>>- * @return 0 if OK, AVERROR_xxx on error
> >>>>>+ * @return 0 if OK, AVERROR(EAGAIN) in case call should be repeated,
> >>>>>+ *         other AVERROR on error
> >>>>>   */
> >>>>>  int av_write_trailer(AVFormatContext *s);
> >>>>>diff --git a/libavformat/mux.c b/libavformat/mux.c
> >>>>>index e9973ed..3ae924c 100644
> >>>>>--- a/libavformat/mux.c
> >>>>>+++ b/libavformat/mux.c
> >>>>>@@ -1204,11 +1204,14 @@ int av_write_trailer(AVFormatContext *s)
> >>>>>      for (;; ) {
> >>>>>          AVPacket pkt;
> >>>>>          ret = interleave_packet(s, &pkt, NULL, 1);
> >>>>>-        if (ret < 0)
> >>>>>-            goto fail;
> >>>>>          if (!ret)
> >>>>>              break;
> >>>>>+        av_assert0(!(s->flags & AVFMT_FLAG_NONBLOCK));
> >>>>this would abort on any error not just EAGAIN
> >>>I think it will abort in case interleave_packets does not return 0
> >>>from the first call in loop, which means that interleaving was used
> >>>(because there are some packets to be flushed) and that situation
> >>>cannot happen with AVFMT_FLAG_NONBLOCK set when interleaving is
> >>>forbidded. The next patch also adds assert to
> >>>av_interleaved_write_packet. But I think the assert here is on the
> >>>right place, or have I misunderstood the problem you're pointing
> >>>out?
> >>I thought interleave_packet can return AVERROR(ENOMEM)
> >>maybe this is not possible, still it seems non-robust to assert if
> >>it errors out
> >>
> >I think it cannot return AVERROR(ENOMEM) in case interleaving was
> >not used. But maybe I can remove assert from this function since
> >there will be assert in av_interleaved_write_frame?
> >
> >Regards,
> >Jan
> Should I remove the assert from the patch (since it will be in
> av_interleaved_write_frame)?

feel free to apply it with or without the assert

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160902/31492fbf/attachment.sig>