| Message ID | 1711536691-15749-2-git-send-email-t.rapp@noa-archive.com |
|---|---|
| State | Accepted |
| Commit | 02eb2fc577e926e9f927829f1d8c8cdb8f31cbbd |
| Headers | show |
| Series | [FFmpeg-devel,1/2] examples/decode_filter_audio: Add loop for draining the filtergraph | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On date Wednesday 2024-03-27 11:51:31 +0100, Tobias Rapp wrote: > Depending on the filters used the filtergraph can produce trailing data > after feeding it the last input frame. Update the example to include the > necessary loop for draining the filtergrap. > > Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > --- > doc/examples/decode_filter_video.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/doc/examples/decode_filter_video.c b/doc/examples/decode_filter_video.c > index 454c192..a57e6df 100644 > --- a/doc/examples/decode_filter_video.c > +++ b/doc/examples/decode_filter_video.c > @@ -276,6 +276,25 @@ int main(int argc, char **argv) > } > av_packet_unref(packet); > } > + if (ret == AVERROR_EOF) { > + /* signal EOF to the filtergraph */ > + if (av_buffersrc_add_frame_flags(buffersrc_ctx, NULL, 0) < 0) { > + av_log(NULL, AV_LOG_ERROR, "Error while closing the filtergraph\n"); > + goto end; > + } > + > + /* pull remaining frames from the filtergraph */ > + while (1) { > + ret = av_buffersink_get_frame(buffersink_ctx, filt_frame); > + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) > + break; how are we supposed to handle the EAGAIN case? Shouldn't this be a sleep and retry?
On 27/03/2024 13:06, Stefano Sabatini wrote: > On date Wednesday 2024-03-27 11:51:31 +0100, Tobias Rapp wrote: >> Depending on the filters used the filtergraph can produce trailing data >> after feeding it the last input frame. Update the example to include the >> necessary loop for draining the filtergrap. >> >> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> >> --- >> doc/examples/decode_filter_video.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/doc/examples/decode_filter_video.c b/doc/examples/decode_filter_video.c >> index 454c192..a57e6df 100644 >> --- a/doc/examples/decode_filter_video.c >> +++ b/doc/examples/decode_filter_video.c >> @@ -276,6 +276,25 @@ int main(int argc, char **argv) >> } >> av_packet_unref(packet); >> } >> + if (ret == AVERROR_EOF) { >> + /* signal EOF to the filtergraph */ >> + if (av_buffersrc_add_frame_flags(buffersrc_ctx, NULL, 0) < 0) { >> + av_log(NULL, AV_LOG_ERROR, "Error while closing the filtergraph\n"); >> + goto end; >> + } >> + >> + /* pull remaining frames from the filtergraph */ >> + while (1) { >> + ret = av_buffersink_get_frame(buffersink_ctx, filt_frame); >> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) >> + break; > how are we supposed to handle the EAGAIN case? Shouldn't this be a > sleep and retry? Good suggestion. I could add something like usleep(100) upon EAGAIN. Will post an updated patch.
Quoting Tobias Rapp (2024-03-27 13:46:40) > On 27/03/2024 13:06, Stefano Sabatini wrote: > > > On date Wednesday 2024-03-27 11:51:31 +0100, Tobias Rapp wrote: > >> Depending on the filters used the filtergraph can produce trailing data > >> after feeding it the last input frame. Update the example to include the > >> necessary loop for draining the filtergrap. > >> > >> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > >> --- > >> doc/examples/decode_filter_video.c | 19 +++++++++++++++++++ > >> 1 file changed, 19 insertions(+) > >> > >> diff --git a/doc/examples/decode_filter_video.c b/doc/examples/decode_filter_video.c > >> index 454c192..a57e6df 100644 > >> --- a/doc/examples/decode_filter_video.c > >> +++ b/doc/examples/decode_filter_video.c > >> @@ -276,6 +276,25 @@ int main(int argc, char **argv) > >> } > >> av_packet_unref(packet); > >> } > >> + if (ret == AVERROR_EOF) { > >> + /* signal EOF to the filtergraph */ > >> + if (av_buffersrc_add_frame_flags(buffersrc_ctx, NULL, 0) < 0) { > >> + av_log(NULL, AV_LOG_ERROR, "Error while closing the filtergraph\n"); > >> + goto end; > >> + } > >> + > >> + /* pull remaining frames from the filtergraph */ > >> + while (1) { > >> + ret = av_buffersink_get_frame(buffersink_ctx, filt_frame); > >> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) > >> + break; > > how are we supposed to handle the EAGAIN case? Shouldn't this be a > > sleep and retry? > > Good suggestion. I could add something like usleep(100) upon EAGAIN. No, EAGAIN from a filter after EOF on all inputs is a bug.
On 27/03/2024 13:53, Anton Khirnov wrote: > Quoting Tobias Rapp (2024-03-27 13:46:40) >> On 27/03/2024 13:06, Stefano Sabatini wrote: >> >>> On date Wednesday 2024-03-27 11:51:31 +0100, Tobias Rapp wrote: >>>> Depending on the filters used the filtergraph can produce trailing data >>>> after feeding it the last input frame. Update the example to include the >>>> necessary loop for draining the filtergrap. >>>> >>>> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> >>>> --- >>>> doc/examples/decode_filter_video.c | 19 +++++++++++++++++++ >>>> 1 file changed, 19 insertions(+) >>>> >>>> diff --git a/doc/examples/decode_filter_video.c b/doc/examples/decode_filter_video.c >>>> index 454c192..a57e6df 100644 >>>> --- a/doc/examples/decode_filter_video.c >>>> +++ b/doc/examples/decode_filter_video.c >>>> @@ -276,6 +276,25 @@ int main(int argc, char **argv) >>>> } >>>> av_packet_unref(packet); >>>> } >>>> + if (ret == AVERROR_EOF) { >>>> + /* signal EOF to the filtergraph */ >>>> + if (av_buffersrc_add_frame_flags(buffersrc_ctx, NULL, 0) < 0) { >>>> + av_log(NULL, AV_LOG_ERROR, "Error while closing the filtergraph\n"); >>>> + goto end; >>>> + } >>>> + >>>> + /* pull remaining frames from the filtergraph */ >>>> + while (1) { >>>> + ret = av_buffersink_get_frame(buffersink_ctx, filt_frame); >>>> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) >>>> + break; >>> how are we supposed to handle the EAGAIN case? Shouldn't this be a >>> sleep and retry? >> Good suggestion. I could add something like usleep(100) upon EAGAIN. > No, EAGAIN from a filter after EOF on all inputs is a bug. Ok. Also from a second look at the example all the other locations where EAGAIN is handled do not perform a retry. So adding that would be orthogonal to this patch, IMHO. Regards, Tobias
On 27/03/2024 14:04, Tobias Rapp wrote: > On 27/03/2024 13:53, Anton Khirnov wrote: > >> Quoting Tobias Rapp (2024-03-27 13:46:40) >>> On 27/03/2024 13:06, Stefano Sabatini wrote: >>> >>>> On date Wednesday 2024-03-27 11:51:31 +0100, Tobias Rapp wrote: >>>>> Depending on the filters used the filtergraph can produce trailing >>>>> data >>>>> after feeding it the last input frame. Update the example to >>>>> include the >>>>> necessary loop for draining the filtergrap. >>>>> >>>>> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> >>>>> --- >>>>> doc/examples/decode_filter_video.c | 19 +++++++++++++++++++ >>>>> 1 file changed, 19 insertions(+) >>>>> >>>>> diff --git a/doc/examples/decode_filter_video.c >>>>> b/doc/examples/decode_filter_video.c >>>>> index 454c192..a57e6df 100644 >>>>> --- a/doc/examples/decode_filter_video.c >>>>> +++ b/doc/examples/decode_filter_video.c >>>>> @@ -276,6 +276,25 @@ int main(int argc, char **argv) >>>>> } >>>>> av_packet_unref(packet); >>>>> } >>>>> + if (ret == AVERROR_EOF) { >>>>> + /* signal EOF to the filtergraph */ >>>>> + if (av_buffersrc_add_frame_flags(buffersrc_ctx, NULL, 0) >>>>> < 0) { >>>>> + av_log(NULL, AV_LOG_ERROR, "Error while closing the >>>>> filtergraph\n"); >>>>> + goto end; >>>>> + } >>>>> + >>>>> + /* pull remaining frames from the filtergraph */ >>>>> + while (1) { >>>>> + ret = av_buffersink_get_frame(buffersink_ctx, >>>>> filt_frame); >>>>> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) >>>>> + break; >>>> how are we supposed to handle the EAGAIN case? Shouldn't this be a >>>> sleep and retry? >>> Good suggestion. I could add something like usleep(100) upon EAGAIN. >> No, EAGAIN from a filter after EOF on all inputs is a bug. > > Ok. Also from a second look at the example all the other locations > where EAGAIN is handled do not perform a retry. So adding that would > be orthogonal to this patch, IMHO. > Applied both patches. Thanks for the feedback! Regards, Tobias
diff --git a/doc/examples/decode_filter_video.c b/doc/examples/decode_filter_video.c index 454c192..a57e6df 100644 --- a/doc/examples/decode_filter_video.c +++ b/doc/examples/decode_filter_video.c @@ -276,6 +276,25 @@ int main(int argc, char **argv) } av_packet_unref(packet); } + if (ret == AVERROR_EOF) { + /* signal EOF to the filtergraph */ + if (av_buffersrc_add_frame_flags(buffersrc_ctx, NULL, 0) < 0) { + av_log(NULL, AV_LOG_ERROR, "Error while closing the filtergraph\n"); + goto end; + } + + /* pull remaining frames from the filtergraph */ + while (1) { + ret = av_buffersink_get_frame(buffersink_ctx, filt_frame); + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) + break; + if (ret < 0) + goto end; + display_frame(filt_frame, buffersink_ctx->inputs[0]->time_base); + av_frame_unref(filt_frame); + } + } + end: avfilter_graph_free(&filter_graph); avcodec_free_context(&dec_ctx);
Depending on the filters used the filtergraph can produce trailing data after feeding it the last input frame. Update the example to include the necessary loop for draining the filtergrap. Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> --- doc/examples/decode_filter_video.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)