Hi Tomasz,
On Fri, Jul 08, 2022 at 01:45:35PM +0900, Tomasz Figa wrote:
> Hi Ezequiel,
>
> On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
> <ezequiel@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > Now that we've transitioned all users to vb2_find_buffer API,
> > remove the unused vb2_find_timestamp().
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ++++-----
> > include/media/videobuf2-v4l2.h | 26 +------------------
> > 2 files changed, 7 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > index 075d24ebf44c..a9696442dfba 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > @@ -625,18 +625,18 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
> > .copy_timestamp = __copy_timestamp,
> > };
> >
> > -int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> > - unsigned int start_idx)
> > +struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp)
> > {
> > unsigned int i;
> >
> > - for (i = start_idx; i < q->num_buffers; i++)
> > + for (i = 0; i < q->num_buffers; i++)
> > if (q->bufs[i]->copied_timestamp &&
> > q->bufs[i]->timestamp == timestamp)
> > - return i;
> > - return -1;
> > + return vb2_get_buffer(q, i);
> > +
> > + return NULL;
> > }
> > -EXPORT_SYMBOL_GPL(vb2_find_timestamp);
> > +EXPORT_SYMBOL_GPL(vb2_find_buffer);
> >
> > /*
> > * vb2_querybuf() - query video buffer information
> > diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> > index 7f9ae5b39b78..5a845887850b 100644
> > --- a/include/media/videobuf2-v4l2.h
> > +++ b/include/media/videobuf2-v4l2.h
> > @@ -62,22 +62,6 @@ struct vb2_v4l2_buffer {
> > #define to_vb2_v4l2_buffer(vb) \
> > container_of(vb, struct vb2_v4l2_buffer, vb2_buf)
> >
> > -/**
> > - * vb2_find_timestamp() - Find buffer with given timestamp in the queue
> > - *
> > - * @q: pointer to &struct vb2_queue with videobuf2 queue.
> > - * @timestamp: the timestamp to find.
> > - * @start_idx: the start index (usually 0) in the buffer array to start
> > - * searching from. Note that there may be multiple buffers
> > - * with the same timestamp value, so you can restart the search
> > - * by setting @start_idx to the previously found index + 1.
> > - *
> > - * Returns the buffer index of the buffer with the given @timestamp, or
> > - * -1 if no buffer with @timestamp was found.
> > - */
> > -int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> > - unsigned int start_idx);
> > -
> > /**
> > * vb2_find_buffer() - Find a buffer with given timestamp
> > *
> > @@ -86,15 +70,7 @@ int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> > *
> > * Returns the buffer with the given @timestamp, or NULL if not found.
> > */
> > -static inline struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q,
> > - u64 timestamp)
> > -{
> > - int index = vb2_find_timestamp(q, timestamp, 0);
> > -
> > - if (index < 0)
> > - return NULL;
> > - return vb2_get_buffer(q, index);
> > -}
> > +struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp);
>
> Was there any specific reason to add it as an inline initially rather
> than just having it close to the final shape from the very beginning?
> Sorry for being picky, but I find it more difficult to review this
> way.
>
Yeah, that makes sense, I'll re-work it for the next iteration.
Thanks,
Ezequiel
> Best regards,
> Tomasz
>
> >
> > int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
> >
> > --
> > 2.34.3
> >
