media: videodev2: add V4L2_FMT_FLAG_NO_SOURCE_CHANGE

PATCH In review Maxime Jourdan 3 participants 7 messages Oct 4, 2018 – Dec 7, 2018

Subsystems (2)

Documentation Media (V4L/DVB)

lore

Share & tools

lore

Maxime Jourdan

author

2018-10-04 13:38 UTC #0 lore raw

PATCH 1/1 media: videodev2: add V4L2_FMT_FLAG_NO_SOURCE_CHANGE

When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.

Add a enum_fmt format flag to tag those specific formats.

Signed-off-by Maxime Jourdan <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0

index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+	for this format.
+
 
 
 Return Value

include/uapi/linux/videodev2.h +3 -2

index 3a65951ca51e..a28acee1cb52 100644
@@ -723,8 +723,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_NO_SOURCE_CHANGE	0x0004
 
 	/* Frame Size and frame rate enumeration */
 /*

Signature · 1 line

2.19.0

Tomasz Figa

2018-10-09 07:59 UTC #1 lore raw

Hi Maxime,

Quoted 33 lines · Maxime Jourdan

When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.

Add a enum_fmt format flag to tag those specific formats.

Signed-off-by Maxime Jourdan <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0

index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+       for this format.
+
 
 
 Return Value

include/uapi/linux/videodev2.h +0 -0

index 3a65951ca51e..a28acee1cb52 100644

include/uapi/linux/videodev2.h struct v4l2_fmtdesc { · line 728

Quoted 8 lines

include/uapi/linux/videodev2.h +3 -2

@@ -723,8 +723,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_NO_SOURCE_CHANGE 0x0004

I think it indeed makes sense. I'd suggest submitting this patch together with the series that adds the affected driver, though, since we'd be otherwise just adding a dead API.

Also, it would be good to refer to it from the Decoder Interface documentation. Depending on which one gets in earlier, you might either want to base on it or I'd add a note myself.

Best regards,
Tomasz

Maxime Jourdan

author

2018-10-09 11:04 UTC #2 lore raw

Hi Tomasz,

Quoted 50 lines · Tomasz Figa

Hi Maxime,
Quoted 33 lines · Maxime Jourdan
When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.
Add a enum_fmt format flag to tag those specific formats.
Signed-off-by Maxime Jourdan <[email protected]>
---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0
index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+       for this format.
+
 
 
 Return Value
include/uapi/linux/videodev2.h +0 -0
index 3a65951ca51e..a28acee1cb52 100644
include/uapi/linux/videodev2.h struct v4l2_fmtdesc { · line 728
Quoted 8 lines
include/uapi/linux/videodev2.h +3 -2
@@ -723,8 +723,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_NO_SOURCE_CHANGE 0x0004
I think it indeed makes sense. I'd suggest submitting this patch together with the series that adds the affected driver, though, since we'd be otherwise just adding a dead API.

Quoted 6 lines

Also, it would be good to refer to it from the Decoder Interface documentation. Depending on which one gets in earlier, you might either want to base on it or I'd add a note myself.
Best regards,
Tomasz

Agreed on both points. If your documentation makes it in before, I'll update it within the patch. We'll have time to sync on that anyway in the coming weeks.

Maxime

Hans Verkuil

2018-11-29 09:01 UTC #3 lore raw

In reply to Maxime Jourdan

Quoted 4 lines · Maxime Jourdan

When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.
Add a enum_fmt format flag to tag those specific formats.

I think I missed (or forgot) some discussion about this since I have no idea why this flag is needed. What's the use-case?

Regards,

Hans

Quoted 40 lines

Signed-off-by Maxime Jourdan <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0

index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+	for this format.
+
 
 
 Return Value

include/uapi/linux/videodev2.h +3 -2

index 3a65951ca51e..a28acee1cb52 100644
@@ -723,8 +723,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_NO_SOURCE_CHANGE	0x0004
 
 	/* Frame Size and frame rate enumeration */
 /*

Tomasz Figa

2018-11-29 19:35 UTC #4 lore raw

Quoted 9 lines · Hans Verkuil

Quoted 4 lines · Maxime Jourdan
When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.
Add a enum_fmt format flag to tag those specific formats.
I think I missed (or forgot) some discussion about this since I have no idea why this flag is needed. What's the use-case?

As far as I remember, the hardware/firmware Maxime has been working with can't handle resolution changes for some coded formats. Perhaps we should explain that better in the commit message and documentation of the flag, though. Maxime, could you refresh my memory with the details?

Best regards,
Tomasz

Quoted 46 lines

Regards,

Hans

Quoted 40 lines

Signed-off-by Maxime Jourdan <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0

index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+     for this format.
+
 
 
 Return Value

include/uapi/linux/videodev2.h +3 -2

index 3a65951ca51e..a28acee1cb52 100644
@@ -723,8 +723,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_NO_SOURCE_CHANGE       0x0004
 
      /* Frame Size and frame rate enumeration */
 /*

Hans Verkuil

2018-11-30 07:35 UTC #5 lore raw

Quoted 8 lines · Tomasz Figa

Quoted 7 lines · Hans Verkuil
Quoted 4 lines · Maxime Jourdan
When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.
Add a enum_fmt format flag to tag those specific formats.

Quoted 8 lines

Quoted 2 lines
I think I missed (or forgot) some discussion about this since I have no idea why this flag is needed. What's the use-case?
As far as I remember, the hardware/firmware Maxime has been working with can't handle resolution changes for some coded formats. Perhaps we should explain that better in the commit message and documentation of the flag, though. Maxime, could you refresh my memory with the details?

So basically it describes if a compressed format can handle resolution changes for the given hardware?

If that's the case, then NO_SOURCE_CHANGE is not a good name as it describes the symptom, not the real reason.

Perhaps _FIXED_RESOLUTION might be a better name.

Regards,

Hans

Quoted 50 lines

Best regards,
Tomasz

Quoted 46 lines

Regards,

Hans

Quoted 40 lines

Signed-off-by Maxime Jourdan <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0

index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+     for this format.
+
 
 
 Return Value

include/uapi/linux/videodev2.h +3 -2

index 3a65951ca51e..a28acee1cb52 100644
@@ -723,8 +723,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_NO_SOURCE_CHANGE       0x0004
 
      /* Frame Size and frame rate enumeration */
 /*

Maxime Jourdan

author

2018-12-07 16:12 UTC #6 lore raw

Hi Hans, Tomasz,

Sorry I missed your messages last week..!

Quoted 13 lines · Hans Verkuil

Quoted 11 lines · Tomasz Figa
Quoted 9 lines · Hans Verkuil
Quoted 4 lines · Maxime Jourdan
When a v4l2 driver exposes V4L2_EVENT_SOURCE_CHANGE, some (usually OUTPUT) formats may not be able to trigger this event.
Add a enum_fmt format flag to tag those specific formats.
I think I missed (or forgot) some discussion about this since I have no idea why this flag is needed. What's the use-case?

Quoted 9 lines

Quoted 5 lines
As far as I remember, the hardware/firmware Maxime has been working with can't handle resolution changes for some coded formats. Perhaps we should explain that better in the commit message and documentation of the flag, though. Maxime, could you refresh my memory with the details?
So basically it describes if a compressed format can handle resolution changes for the given hardware?

Yes, exactly

Quoted 5 lines

If that's the case, then NO_SOURCE_CHANGE is not a good name as it describes the symptom, not the real reason.
Perhaps _FIXED_RESOLUTION might be a better name.

Fair point, I will update it as such.

Next time, this patch will come in a series that uses the flag.

Cheers,
Maxime

Quoted 55 lines

Regards,

Hans

Quoted 50 lines

Best regards,
Tomasz

Quoted 46 lines

Regards,

Hans

Quoted 40 lines

Signed-off-by Maxime Jourdan <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 5 +++++
 include/uapi/linux/videodev2.h                   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Documentation/media/uapi/v4l/vidioc-enum-fmt.rst +5 -0

index 019c513df217..e0040b36ac43 100644
@@ -116,6 +116,11 @@ 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_NO_SOURCE_CHANGE``
+      - 0x0004
+      - The event ``V4L2_EVENT_SOURCE_CHANGE`` is not supported
+     for this format.
+
 
 
 Return Value

include/uapi/linux/videodev2.h +3 -2

index 3a65951ca51e..a28acee1cb52 100644
@@ -723,8 +723,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_NO_SOURCE_CHANGE       0x0004
 
      /* Frame Size and frame rate enumeration */
 /*