| Message ID | 20161229143403.2851-12-george@nsup.org |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Dec 29, 2016 at 03:33:58PM +0100, Nicolas George wrote: > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/avfilter.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > Unchanged. slightly off topic but ff_update_link_current_pts() has no documentation in the header also the commit message is a bit terse, why is it moved does this add or remove calls to it in some codepathes ? [...]
Le primidi 11 nivôse, an CCXXV, Michael Niedermayer a écrit : > slightly off topic but ff_update_link_current_pts() has no > documentation in the header I also noticed it. Also, it is now only used in avfilter.c, so it could be made static. I will have to address it when I get to maintaining a priority queue of filters that are ready and deprecating avfilter_graph_request_oldest(), since they are related (ff_update_link_current_pts() maintains a heap of sink links by timestamp). > also the commit message is a bit terse, why is it moved > does this add or remove calls to it in some codepathes ? I reworded it as: lavfi: move ff_update_link_current_pts() into the utility functions. It does not change anything for the existing filters and makes better code fatrorization when future code will use the utility functions. My reason to have it in a separate commit is to be able to run FATE before and after the change to check it did not break anything. Regards,
On Sat, Dec 31, 2016 at 02:29:45PM +0100, Nicolas George wrote: > Le primidi 11 nivôse, an CCXXV, Michael Niedermayer a écrit : > > slightly off topic but ff_update_link_current_pts() has no > > documentation in the header > > I also noticed it. Also, it is now only used in avfilter.c, so it could > be made static. I will have to address it when I get to maintaining a > priority queue of filters that are ready and deprecating > avfilter_graph_request_oldest(), since they are related > (ff_update_link_current_pts() maintains a heap of sink links by > timestamp). > > > also the commit message is a bit terse, why is it moved > > does this add or remove calls to it in some codepathes ? > > I reworded it as: > > lavfi: move ff_update_link_current_pts() into the utility functions. > > It does not change anything for the existing filters and makes > better code fatrorization when future code will use the utility > functions. > > My reason to have it in a separate commit is to be able to run FATE > before and after the change to check it did not break anything. LGTM thx [...]
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 715ecb3300..7e02d9ca8b 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -1116,7 +1116,6 @@ static int ff_filter_frame_framed(AVFilterLink *link, AVFrame *frame) filter_frame = default_filter_frame; ret = filter_frame(link, frame); link->frame_count_out++; - ff_update_link_current_pts(link, frame->pts); return ret; fail: @@ -1515,6 +1514,7 @@ int ff_inlink_check_available_samples(AVFilterLink *link, unsigned min) static void consume_update(AVFilterLink *link, const AVFrame *frame) { + ff_update_link_current_pts(link, frame->pts); ff_inlink_process_commands(link, frame); ff_inlink_process_timeline(link, frame); link->frame_count_out++;
Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/avfilter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Unchanged.