From: "\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH v2] fftools/ffmpeg_mux: fix reporting muxer EOF as error Date: Sun, 23 Apr 2023 17:48:24 +0800 Message-ID: <tencent_5B20CD38A03DCDEFCFE3D52AA76A38B59607@qq.com> (raw) In-Reply-To: <ae7843ac-e742-9b3c-f699-3eb7b6b2585a@passwd.hu> > On Apr 23, 2023, at 17:42, Marton Balint <cus@passwd.hu> wrote: > > On Sun, 23 Apr 2023, Anton Khirnov wrote: > >> Quoting Marton Balint (2023-04-23 11:12:38) >>> >>> >>> On Sat, 22 Apr 2023, Anton Khirnov wrote: >>> >>>> Quoting Zhao Zhili (2023-04-22 14:56:34) >>>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>>> >>>>> Fix #10327. >>>>> >>>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>>>> --- >>>>> fftools/ffmpeg_mux.c | 12 +++++++++--- >>>>> 1 file changed, 9 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c >>>>> index a2e8873ad2..0e1a5d7dc5 100644 >>>>> --- a/fftools/ffmpeg_mux.c >>>>> +++ b/fftools/ffmpeg_mux.c >>>>> @@ -214,9 +214,15 @@ static void *muxer_thread(void *arg) >>>>> ost = of->streams[stream_idx]; >>>>> ret = sync_queue_process(mux, ost, ret < 0 ? NULL : pkt, &stream_eof); >>>>> av_packet_unref(pkt); >>>>> - if (ret == AVERROR_EOF && stream_eof) >>>>> - tq_receive_finish(mux->tq, stream_idx); >>>>> - else if (ret < 0) { >>>>> + if (ret == AVERROR_EOF) { >>>>> + if (stream_eof) { >>>>> + tq_receive_finish(mux->tq, stream_idx); >>>>> + } else { >>>>> + av_log(mux, AV_LOG_VERBOSE, "Muxer %s\n", av_err2str(ret)); >>>> >>>> That seems unnecesarily convoluted, given that we _know_ the error to be >>>> EOF here. Also, please make it "Muxer returned EOF" to make it clear >>>> what exactly is the source of EOF. >>>> >>>> Otherwise ok, feel free to push with this change. >>> >>> This seems like yet another clash of AVERROR_EOF error codes coming from >>> different places with different semantics. For >>> av_interleaved_write_frame(), AVERROR_EOF is an error condition, so >>> file encoding should fail, >> >> Why should it fail? I'd think a muxer returning EOF is the way to signal >> non-error muxer-side termination. > > That would be an API change. AVERROR_EOF is not special in any way from other error codes for av_interleaved_write_frame. A muxer cannot signal non-error muxer side termination with existing API. Sorry I have pushed before seeing your comments. It can happen in theory, but I’m not sure in which case muxer can return EOF and should be treated as a real error condition. A simple grep shows: $ grep 'AVERROR_EOF' libavformat/*enc.c libavformat/hlsenc.c: if (ret >= 0 || ret == AVERROR_EOF) <--- It changed ret to 0 afterwards. libavformat/webmdashenc.c: return AVERROR_EOF; <—-- More like a bug on the caller side than a real error. > > Regards, > Marton > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2023-04-23 9:48 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-04-22 12:56 Zhao Zhili 2023-04-22 15:44 ` Anton Khirnov 2023-04-23 9:12 ` Marton Balint 2023-04-23 9:34 ` Anton Khirnov 2023-04-23 9:42 ` Marton Balint 2023-04-23 9:48 ` "zhilizhao(赵志立)" [this message] 2023-04-23 9:51 ` Anton Khirnov 2023-04-23 10:05 ` Marton Balint 2023-04-23 10:07 ` Nicolas George 2023-04-23 12:01 ` Anton Khirnov 2023-04-23 18:15 ` Marton Balint 2023-04-23 20:16 ` Anton Khirnov 2023-04-24 9:09 ` Marton Balint 2023-04-24 9:19 ` Nicolas George 2023-04-24 10:02 ` Anton Khirnov 2023-04-24 18:41 ` Marton Balint 2023-04-24 19:24 ` Anton Khirnov 2023-04-24 19:42 ` Marton Balint 2023-04-24 21:08 ` Anton Khirnov 2023-04-25 14:37 ` Nicolas George 2023-04-24 20:41 ` Nicolas George 2023-04-24 10:08 ` Anton Khirnov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=tencent_5B20CD38A03DCDEFCFE3D52AA76A38B59607@qq.com \ --to=quinkblack@foxmail.com \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git