linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers
@ 2019-01-07 11:30 Hans Verkuil
  2019-01-07 11:36 ` Hans Verkuil
  0 siblings, 1 reply; 4+ messages in thread
From: Hans Verkuil @ 2019-01-07 11:30 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Paul Kocialkowski, Maxime Ripard, Tomasz Figa

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.

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(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers
  2019-01-07 11:30 [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers Hans Verkuil
@ 2019-01-07 11:36 ` Hans Verkuil
  2019-01-08  9:16   ` Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers[Please note,mail behalf by linux-media-owner@vger.kernel.org] Randy Li
       [not found]   ` <1468691710.6518623.1546938541229.JavaMail.javamailuser@localhost>
  0 siblings, 2 replies; 4+ messages in thread
From: Hans Verkuil @ 2019-01-07 11:36 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Paul Kocialkowski, Maxime Ripard, Tomasz Figa

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(-)
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers[Please note,mail behalf by linux-media-owner@vger.kernel.org]
  2019-01-07 11:36 ` Hans Verkuil
@ 2019-01-08  9:16   ` Randy Li
       [not found]   ` <1468691710.6518623.1546938541229.JavaMail.javamailuser@localhost>
  1 sibling, 0 replies; 4+ messages in thread
From: Randy Li @ 2019-01-08  9:16 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Paul Kocialkowski, Maxime Ripard,
	Tomasz Figa, Randy Li

Please don't do that


I reply the

Re: [PATCHv4 00/10] As was discussed here (among other places):

talking about the disadvantage of using the buffer tag, driver won't 
aware which buffer will be used for current picture unless all the 
previous  work are done.

On 1/7/19 7:36 PM, Hans Verkuil wrote:
> 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(-)
>>



^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers
       [not found]   ` <1468691710.6518623.1546938541229.JavaMail.javamailuser@localhost>
@ 2019-01-08  9:59     ` Hans Verkuil
  0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2019-01-08  9:59 UTC (permalink / raw)
  To: 李夏潤 (Randy Li), Linux Media Mailing List
  Cc: Paul Kocialkowski, Maxime Ripard, Tomasz Figa

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(-)
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-08  9:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 11:30 [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers Hans Verkuil
2019-01-07 11:36 ` Hans Verkuil
2019-01-08  9:16   ` Re: [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers[Please note,mail behalf by linux-media-owner@vger.kernel.org] Randy Li
     [not found]   ` <1468691710.6518623.1546938541229.JavaMail.javamailuser@localhost>
2019-01-08  9:59     ` [GIT PULL FOR v5.1] vb2/cedrus: use timestamps to identify buffers Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).