diff mbox series

[v7,2/4] media: videodev2: add V4L2_FMT_FLAG_FIXED_RESOLUTION

Message ID 20190531093126.26956-3-mjourdan@baylibre.com (mailing list archive)
State New, archived
Headers show
Series Add Amlogic video decoder driver | expand

Commit Message

Maxime Jourdan May 31, 2019, 9:31 a.m. UTC
When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually
OUTPUT) formats may not be able to trigger this event.

For instance, MPEG2 on Amlogic hardware does not support resolution
switching on the fly, and a decode session must operate at a set
resolution defined before the decoding start.

Add a enum_fmt format flag to tag those specific formats.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 6 ++++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Hans Verkuil June 5, 2019, 1:39 p.m. UTC | #1
Hi Maxime,

I am wondering if this flag shouldn't be inverted: you set
V4L2_FMT_FLAG_DYN_RESOLUTION if dynamic resolution is supported,
otherwise it isn't.

Can all the existing mainlined codec drivers handle midstream
resolution changes?

s5p-mfc, venus and mediatek can, but I see no SOURCE_CHANGE event in
the coda drivers, so I suspect that that can't handle this.

Philipp, what is the status of the coda driver for dynamic resolution
changes?

Another reason is that AFAIK all encoders are of the fixed resolution
type.

And a final reason is that usually flags should indicate the presence
of a feature, not the absence.

Regards,

	Hans

On 5/31/19 11:31 AM, Maxime Jourdan wrote:
> When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually
> OUTPUT) formats may not be able to trigger this event.
> 
> For instance, MPEG2 on Amlogic hardware does not support resolution
> switching on the fly, and a decode session must operate at a set
> resolution defined before the decoding start.
> 
> Add a enum_fmt format flag to tag those specific formats.
> 
> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> ---
>  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 6 ++++++
>  include/uapi/linux/videodev2.h                   | 5 +++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> index 822d6730e7d2..b11448a1848b 100644
> --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> @@ -127,6 +127,12 @@ one until ``EINVAL`` is returned.
>        - This format is not native to the device but emulated through
>  	software (usually libv4l2), where possible try to use a native
>  	format instead for better performance.
> +    * - ``V4L2_FMT_FLAG_FIXED_RESOLUTION``
> +      - 0x0004
> +      - Dynamic resolution switching is not supported for this format,
> +        even if the event ``V4L2_EVENT_SOURCE_CHANGE`` is supported by
> +        the device.
> +
>  
>  
>  Return Value
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 1050a75fb7ef..9b0a7f82dd92 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -768,8 +768,9 @@ struct v4l2_fmtdesc {
>  	__u32		    reserved[4];
>  };
>  
> -#define V4L2_FMT_FLAG_COMPRESSED 0x0001
> -#define V4L2_FMT_FLAG_EMULATED   0x0002
> +#define V4L2_FMT_FLAG_COMPRESSED	0x0001
> +#define V4L2_FMT_FLAG_EMULATED		0x0002
> +#define V4L2_FMT_FLAG_FIXED_RESOLUTION	0x0004
>  
>  	/* Frame Size and frame rate enumeration */
>  /*
>
Philipp Zabel June 11, 2019, 8:52 a.m. UTC | #2
On Wed, 2019-06-05 at 15:39 +0200, Hans Verkuil wrote:
> Hi Maxime,
> 
> I am wondering if this flag shouldn't be inverted: you set
> V4L2_FMT_FLAG_DYN_RESOLUTION if dynamic resolution is supported,
> otherwise it isn't.
> 
> Can all the existing mainlined codec drivers handle midstream
> resolution changes?
> 
> s5p-mfc, venus and mediatek can, but I see no SOURCE_CHANGE event in
> the coda drivers, so I suspect that that can't handle this.
> 
> Philipp, what is the status of the coda driver for dynamic resolution
> changes?

FTR, to my knowledge there is no dynamic resolution change support in
the firmware, as there is no signal (interrupt nor picture run return
value) to indicate that different headers were parsed.

I am planning to add the initial source change event required by the
current decoder API documentation, but I am afraid there will be no
support for source changes due to mid-stream resolution changes due to
firmware limitations.

regards
Philipp
Nicolas Dufresne June 12, 2019, 12:58 a.m. UTC | #3
Le mardi 11 juin 2019 à 10:52 +0200, Philipp Zabel a écrit :
> On Wed, 2019-06-05 at 15:39 +0200, Hans Verkuil wrote:
> > Hi Maxime,
> > 
> > I am wondering if this flag shouldn't be inverted: you set
> > V4L2_FMT_FLAG_DYN_RESOLUTION if dynamic resolution is supported,
> > otherwise it isn't.
> > 
> > Can all the existing mainlined codec drivers handle midstream
> > resolution changes?
> > 
> > s5p-mfc, venus and mediatek can, but I see no SOURCE_CHANGE event in
> > the coda drivers, so I suspect that that can't handle this.
> > 
> > Philipp, what is the status of the coda driver for dynamic resolution
> > changes?
> 
> FTR, to my knowledge there is no dynamic resolution change support in
> the firmware, as there is no signal (interrupt nor picture run return
> value) to indicate that different headers were parsed.
> 
> I am planning to add the initial source change event required by the
> current decoder API documentation, but I am afraid there will be no
> support for source changes due to mid-stream resolution changes due to
> firmware limitations.

I'm far from familiar with this IP, but at least on CODA988, I can read
from the manual that the workflow is to first guess the allocation, and
if you guess it wrong, an error is returned. What seems to match the
SOURCE_CHANGE event in that version would be in the picture status
register, the bit 20, which is documented as triggered if the stream
requires bigger buffers sizes, or more buffers. After fixing that, you
should, if I read correctly, retry.

It does not notify if the buffers are too large, but you can detect,
since there is register with the output stream information. This
basically means that for V4L2 restriction, you'd have to bounce the
buffers on frame size boundary or something like thisé

This workflow is very similar to how OMX works, but V4L2 is even less
flexible on allocation vs format, forcing more re-allocation.

> 
> regards
> Philipp
Nicolas Dufresne June 12, 2019, 1 a.m. UTC | #4
Le mardi 11 juin 2019 à 20:58 -0400, Nicolas Dufresne a écrit :
> Le mardi 11 juin 2019 à 10:52 +0200, Philipp Zabel a écrit :
> > On Wed, 2019-06-05 at 15:39 +0200, Hans Verkuil wrote:
> > > Hi Maxime,
> > > 
> > > I am wondering if this flag shouldn't be inverted: you set
> > > V4L2_FMT_FLAG_DYN_RESOLUTION if dynamic resolution is supported,
> > > otherwise it isn't.
> > > 
> > > Can all the existing mainlined codec drivers handle midstream
> > > resolution changes?
> > > 
> > > s5p-mfc, venus and mediatek can, but I see no SOURCE_CHANGE event in
> > > the coda drivers, so I suspect that that can't handle this.
> > > 
> > > Philipp, what is the status of the coda driver for dynamic resolution
> > > changes?
> > 
> > FTR, to my knowledge there is no dynamic resolution change support in
> > the firmware, as there is no signal (interrupt nor picture run return
> > value) to indicate that different headers were parsed.
> > 
> > I am planning to add the initial source change event required by the
> > current decoder API documentation, but I am afraid there will be no
> > support for source changes due to mid-stream resolution changes due to
> > firmware limitations.
> 
> I'm far from familiar with this IP, but at least on CODA988, I can read
> from the manual that the workflow is to first guess the allocation, and
> if you guess it wrong, an error is returned. What seems to match the
> SOURCE_CHANGE event in that version would be in the picture status
> register, the bit 20, which is documented as triggered if the stream
> requires bigger buffers sizes, or more buffers. After fixing that, you
> should, if I read correctly, retry.
> 
> It does not notify if the buffers are too large, but you can detect,
> since there is register with the output stream information. This
> basically means that for V4L2 restriction, you'd have to bounce the
> buffers on frame size boundary or something like thisé
> 
> This workflow is very similar to how OMX works, but V4L2 is even less
> flexible on allocation vs format, forcing more re-allocation.

Oh, actually, you already bounce the buffers through the color
converter, so I guess this can all be hidden internally.

> 
> > regards
> > Philipp
diff mbox series

Patch

diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
index 822d6730e7d2..b11448a1848b 100644
--- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
@@ -127,6 +127,12 @@  one until ``EINVAL`` is returned.
       - This format is not native to the device but emulated through
 	software (usually libv4l2), where possible try to use a native
 	format instead for better performance.
+    * - ``V4L2_FMT_FLAG_FIXED_RESOLUTION``
+      - 0x0004
+      - Dynamic resolution switching is not supported for this format,
+        even if the event ``V4L2_EVENT_SOURCE_CHANGE`` is supported by
+        the device.
+
 
 
 Return Value
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 1050a75fb7ef..9b0a7f82dd92 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -768,8 +768,9 @@  struct v4l2_fmtdesc {
 	__u32		    reserved[4];
 };
 
-#define V4L2_FMT_FLAG_COMPRESSED 0x0001
-#define V4L2_FMT_FLAG_EMULATED   0x0002
+#define V4L2_FMT_FLAG_COMPRESSED	0x0001
+#define V4L2_FMT_FLAG_EMULATED		0x0002
+#define V4L2_FMT_FLAG_FIXED_RESOLUTION	0x0004
 
 	/* Frame Size and frame rate enumeration */
 /*