Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers

[Hans Verkuil <hverkuil@xs4all.nl>]

On 01/08/19 10:09, 李夏潤 (Randy Li) wrote:
> I sent the other talking about the disadvantage of using buffer tag in
> 
> Re: [PATCHv4 00/10] As was discussed here (among other places)
> 
> 
> I agree with the buffer tag. But coping from the OUTPUT side to CAPTURE side is a bad idea.
> 
> I think we need a method to refer any buffers when they are allocated.
> 
> When I push a slice with its parameters into the driver, its previous picture in decoded order may not ready yet, using the buffer index, the driver
> 
> is still able to generate the registers table for it. Although you may though it just an additional buffer assignment work before wrote it into the device,
> 
> a few times seeking a buffer in a list. But there is a mode in new generation Rockchip device, called the link mode, you can put a registers into a memory, device would process that register link. You can't interrupt it. That is pretty useful for those codec converting.
> 
> 
> Besides, I found it is little hard to refer a buffer with different offsets at the same time, it would be used for multiple slices and multiple CTU or filed picture which are not usual case nowadays.
> 
> 
> Please don't do that.

To be honest, I don't understand what the problem is. Can you explain in more
detail, perhaps with an example?

Does anyone else understand this? Tomasz, I think you've worked with rockchip before, do
you see what Randy is referring to?

Regards,

	Hans

> 
> 
> Randy Li
> 
> *From:* "Hans Verkuil <hverkuil@xs4all.nl>"
> 
> *To:* "Linux Media Mailing List <linux-media@vger.kernel.org>"
> 
> *CC:* "Paul Kocialkowski <paul.kocialkowski@bootlin.com>","Maxime Ripard <maxime.ripard@bootlin.com>","Tomasz Figa <tfiga@chromium.org>"
> 
> *Sent:* 2019-01-07 19:36
> 
> *Subject:* Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers[Please note,mail behalf by linux-media-owner@vger.kernel.org]
> 
> On 01/07/2019 12:30 PM, Hans Verkuil wrote:
>> As was discussed here (among other places):
>>
>>https://lkml.org/lkml/2018/10/19/440
>>
>> using capture queue buffer indices to refer to reference frames is
>> not a good idea.
>>
>> Instead, after a long irc discussion:
>>
>>https://linuxtv.org/irc/irclogger_log/v4l?date=2018-12-12,Wed
>>
>> it was decided to use the timestamp in v4l2_buffer for this.
>>
>> However, struct timeval cannot be used in a compound control since
>> the size of struct timeval differs between 32 and 64 bit architectures,
>> and there are also changes upcoming for y2038 support.
>>
>> But internally the kernel converts the timeval to a u64 (nsecs since
>> boot). So we provide a helper function in videodev2.h that converts
>> the timeval to a u64, and that u64 can be used inside compound controls.
>>
>> In the not too distant future we want to create a new struct v4l2_buffer,
>> and then we'll use u64 from the start, so in that case the helper function
>> would no longer be needed.
>>
>> The first three patches add a new m2m helper function to correctly copy
>> the relevant data from an output buffer to a capture buffer. This will
>> simplify m2m drivers (in fact, many m2m drivers do not do this quite
>> right, so a helper function was really needed).
>>
>> The fourth patch clears up messy timecode documentation that I came
>> across while working on this.
>>
>> Patch 5 adds the new v4l2_timeval_to_ns helper function to videodev2.h.
>> The next patch adds the vb2_find_timestamp() function to find buffers
>> with a specific u64 timestamp.
>>
>> Finally the cedrus driver and documentation are updated to use a
>> timestamp as buffer identifier.
>>
>> I also removed the 'pad' fields from the mpeg2 control structs (it
>> should never been added in the first place) and aligned the structs
>> to a u32 boundary.
> 
> Note that this pull request corresponds with the v6 patch series.
> ("[PATCHv6 0/8] vb2/cedrus: use timestamps to identify buffers")
> 
> Regards,
> Hans
> 
>>
>> Regards,
>>
>>         Hans
>>
>> The following changes since commit 4bd46aa0353e022c2401a258e93b107880a66533:
>>
>>   media: cx23885: only reset DMA on problematic CPUs (2018-12-20 06:52:01 -0500)
>>
>> are available in the Git repository at:
>>
>>   git://linuxtv.org/hverkuil/media_tree.git tags/br-buftag
>>
>> for you to fetch changes up to 690da7b0ab96f6761e72bb0c5c861e1e13acb327:
>>
>>   extended-controls.rst: update the mpeg2 compound controls (2019-01-07 12:23:49 +0100)
>>
>> ----------------------------------------------------------------
>> Tag branch
>>
>> ----------------------------------------------------------------
>> Hans Verkuil (8):
>>       v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
>>       vim2m: use v4l2_m2m_buf_copy_data
>>       vicodec: use v4l2_m2m_buf_copy_data
>>       buffer.rst: clean up timecode documentation
>>       videodev2.h: add v4l2_timeval_to_ns inline function
>>       vb2: add vb2_find_timestamp()
>>       cedrus: identify buffers by timestamp
>>       extended-controls.rst: update the mpeg2 compound controls
>>
>>  Documentation/media/uapi/v4l/buffer.rst            | 11 +++++------
>>  Documentation/media/uapi/v4l/extended-controls.rst | 28 +++++++++++++++++-----------
>>  drivers/media/common/videobuf2/videobuf2-v4l2.c    | 19 ++++++++++++++++++-
>>  drivers/media/platform/vicodec/vicodec-core.c      | 12 +-----------
>>  drivers/media/platform/vim2m.c                     | 12 +-----------
>>  drivers/media/v4l2-core/v4l2-ctrls.c               |  9 ---------
>>  drivers/media/v4l2-core/v4l2-mem2mem.c             | 20 ++++++++++++++++++++
>>  drivers/staging/media/sunxi/cedrus/cedrus.h        |  9 ++++++---
>>  drivers/staging/media/sunxi/cedrus/cedrus_dec.c    |  2 ++
>>  drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c  | 23 +++++++++++------------
>>  include/media/mpeg2-ctrls.h                        | 14 +++++---------
>>  include/media/v4l2-mem2mem.h                       | 20 ++++++++++++++++++++
>>  include/media/videobuf2-v4l2.h                     | 17 +++++++++++++++++
>>  include/uapi/linux/videodev2.h                     | 12 ++++++++++++
>>  14 files changed, 135 insertions(+), 73 deletions(-)
>>
>