| Message ID | 20200603133919.8560-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/avfilter: add sample_count_in and sample_count_out | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Paul B Mahol (12020-06-03): > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > To be used by latency filters. Re-submit when something use them. > --- > libavfilter/avfilter.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h > index 49b4f7a939..8ebcb3eacc 100644 > --- a/libavfilter/avfilter.h > +++ b/libavfilter/avfilter.h > @@ -582,6 +582,11 @@ struct AVFilterLink { > */ > int64_t frame_count_in, frame_count_out; > > + /** > + * Number of past samples sent through the link. "Number of samples already sent through the link" seems more natural. > + */ > + int64_t sample_count_in, sample_count_out; Should be uint64_t. > + > /** > * A pointer to a FFFramePool struct. > */
On 6/3/20, Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12020-06-03): >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- > >> To be used by latency filters. > > Re-submit when something use them. Why so? Just to be beaten again by same person which would reject this same patch. Low effort patches are better because author will not lose much time in pointless discussion with same person again and again. Instead patch is either accepted or rejected and than author can concentrate on more important stuff. The idea behind this patch is to use those counters to report via filter metadata current latency in filtergraph. Eg. (a)latency filter would be inserted in filtergraph and then it would report latency between some points in filtergraph, or just single filter before it. If all reviewers are fond of this idea then OK, if not I have other things to do and can not lose precious time in timeless pointless discussion with same bitter persons again and again. > >> --- >> libavfilter/avfilter.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h >> index 49b4f7a939..8ebcb3eacc 100644 >> --- a/libavfilter/avfilter.h >> +++ b/libavfilter/avfilter.h >> @@ -582,6 +582,11 @@ struct AVFilterLink { >> */ >> int64_t frame_count_in, frame_count_out; >> >> + /** > >> + * Number of past samples sent through the link. > > "Number of samples already sent through the link" seems more natural. I copy pasted sentence for frames above. Should it be changed too? > >> + */ > >> + int64_t sample_count_in, sample_count_out; > > Should be uint64_t. Above frame counters are not unsigned. Why to do different. > >> + >> /** >> * A pointer to a FFFramePool struct. >> */ > > -- > Nicolas George >
Paul B Mahol (12020-06-03): > Why so? Just to be beaten again by same person which would reject this > same patch. I was not the first one to reject a patch because in introduces APIs or fields without users, but I think it is a good policy. > > "Number of samples already sent through the link" seems more natural. > I copy pasted sentence for frames above. Should it be changed too? I do not think it matters much. > > Above frame counters are not unsigned. Why to do different. If I had noticed it when they were added, I would have suggested they be unsigned too. Signed arithmetic is more tricky, and compiler optimize it less well: if something does not need to be negative, let it be unsigned.
On 6/3/2020 2:27 PM, Nicolas George wrote: > Paul B Mahol (12020-06-03): >> Why so? Just to be beaten again by same person which would reject this >> same patch. > > I was not the first one to reject a patch because in introduces APIs or > fields without users, but I think it is a good policy. > >>> "Number of samples already sent through the link" seems more natural. >> I copy pasted sentence for frames above. Should it be changed too? > > I do not think it matters much. >> >> Above frame counters are not unsigned. Why to do different. > > If I had noticed it when they were added, I would have suggested they be > unsigned too. Signed arithmetic is more tricky, and compiler optimize it > less well: if something does not need to be negative, let it be > unsigned. None of these fields are public or meant to be accessed by any of the other libraries, so they can be changed as needed. Feel free to make them unsigned if you think it's worth it or more robust. Just ensure you're not introducing bugs by doing so. A quick grep shows things like FFMAX(0, outlink->frame_count_in - 1).
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h index 49b4f7a939..8ebcb3eacc 100644 --- a/libavfilter/avfilter.h +++ b/libavfilter/avfilter.h @@ -582,6 +582,11 @@ struct AVFilterLink { */ int64_t frame_count_in, frame_count_out; + /** + * Number of past samples sent through the link. + */ + int64_t sample_count_in, sample_count_out; + /** * A pointer to a FFFramePool struct. */
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- To be used by latency filters. --- libavfilter/avfilter.h | 5 +++++ 1 file changed, 5 insertions(+)