linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] Add LTR controls
@ 2020-06-11 10:25 Dikshita Agarwal
  2020-06-11 10:25 ` [RFC PATCH 1/1] media: v4l2-ctrls: add control for ltr Dikshita Agarwal
  2020-06-11 14:22 ` [RFC PATCH 0/1] Add LTR controls Nicolas Dufresne
  0 siblings, 2 replies; 11+ messages in thread
From: Dikshita Agarwal @ 2020-06-11 10:25 UTC (permalink / raw)
  To: mchehab, hverkuil-cisco, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel
  Cc: linux-arm-msm, vgarodia, majja, Dikshita Agarwal

LTR (Long Term Reference) frames are the frames that are encoded sometime in the past
and stored in the DPB buffer list to be used as reference to encode future frames.
One usage of LTR encoding is to reduce error propagation for video transmission
in packet lossy networks.  For example, encoder may want to specify some key frames as
LTR pictures and use them as reference frames for encoding. With extra protection
selectively on these LTR frames or synchronization with the receiver of reception of
the LTR frames during transmission, decoder can receive reference frames more reliably
than other non-reference frames. As a result, transmission error can be effectively
restricted within certain frames rather than propagated to future frames.

We are introducing below V4l2 Controls for this feature
1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
    a. This is used to query or configure the number of LTR frames.
       This is a static control and is controlled by the client.
    b. The LTR index varies from 0 to the max LTR-1.
    c. If LTR Count is more than max supported LTR count (max LTR) by driver, it will be rejected.
    d. Auto Marking : If LTR count is non zero,
        1) first LTR count frames would be mark as LTR automatically after
      	   every IDR frame (inclusive).
        2) For multilayer encoding: first LTR count base layer reference frames starting after
           every IDR frame (inclusive) in encoding order would be marked as LTR frames by the encoder.
        3) Auto marking of LTR due to IDR should consider following conditions:
            1. The frame is not already set to be marked as LTR.
            2. The frame is part of the base layer in the hierarchical layer case.
            3. The number of frames currently marked as LTR is less than the maximum LTR frame index plus 1.
    e. Encoder needs to handle explicit Mark/Use command when encoder is still doing "auto" marking

2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
    a. This signals to mark the current frame as LTR frame. It is a dynamic control and also provide the LTR index to be used.
    b. the LTR index provided by this control should never exceed the max LTR-1. Else it will be rejected.

3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
    a. This specifies the LTR frame(s) to be used for encoding the current frame. This is a dynamic control.
    b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N LSB bits of this field are valid,
       where N is the maximum number of LTRs supported. All the other bits are invalid and should be rejected.
       The LSB corresponds to the LTR index 0. Bit N-1 from the LSB corresponds to the LTR index max LTR-1.

Dikshita Agarwal (1):
  media: v4l2-ctrls:  add control for ltr

 drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
 include/uapi/linux/v4l2-controls.h   | 4 ++++
 2 files changed, 10 insertions(+)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 1/1] media: v4l2-ctrls: add control for ltr
  2020-06-11 10:25 [RFC PATCH 0/1] Add LTR controls Dikshita Agarwal
@ 2020-06-11 10:25 ` Dikshita Agarwal
  2020-06-11 14:22 ` [RFC PATCH 0/1] Add LTR controls Nicolas Dufresne
  1 sibling, 0 replies; 11+ messages in thread
From: Dikshita Agarwal @ 2020-06-11 10:25 UTC (permalink / raw)
  To: mchehab, hverkuil-cisco, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel
  Cc: linux-arm-msm, vgarodia, majja, Dikshita Agarwal

Add v4l2 controls for ltr

Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
 include/uapi/linux/v4l2-controls.h   | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 93d33d1..205863e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -977,6 +977,9 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";
 	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";
+	case V4L2_CID_MPEG_VIDEO_LTRCOUNT: 			return "LTR Count";
+	case V4L2_CID_MPEG_VIDEO_MARKLTRFRAME: 			return "Mark LTR";
+	case V4L2_CID_MPEG_VIDEO_USELTRFRAME:			return "Use LTR";
 
 	/* CAMERA controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1208,6 +1211,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
 	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
+	case V4L2_CID_MPEG_VIDEO_LTRCOUNT:
+	case V4L2_CID_MPEG_VIDEO_MARKLTRFRAME:
+	case V4L2_CID_MPEG_VIDEO_USELTRFRAME:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
 	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 1a58d7c..88d24e1 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -737,6 +737,10 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field {
 #define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR	(V4L2_CID_MPEG_BASE + 642)
 #define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES	(V4L2_CID_MPEG_BASE + 643)
 #define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR	(V4L2_CID_MPEG_BASE + 644)
+#define V4L2_CID_MPEG_VIDEO_LTRCOUNT			(V4L2_CID_MPEG_BASE + 645)
+#define V4L2_CID_MPEG_VIDEO_MARKLTRFRAME		(V4L2_CID_MPEG_BASE + 646)
+#define V4L2_CID_MPEG_VIDEO_USELTRFRAME		(V4L2_CID_MPEG_BASE + 647)
+
 
 /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
 #define V4L2_CID_MPEG_CX2341X_BASE				(V4L2_CTRL_CLASS_MPEG | 0x1000)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-06-11 10:25 [RFC PATCH 0/1] Add LTR controls Dikshita Agarwal
  2020-06-11 10:25 ` [RFC PATCH 1/1] media: v4l2-ctrls: add control for ltr Dikshita Agarwal
@ 2020-06-11 14:22 ` Nicolas Dufresne
  2020-06-12  9:11   ` Hans Verkuil
  1 sibling, 1 reply; 11+ messages in thread
From: Nicolas Dufresne @ 2020-06-11 14:22 UTC (permalink / raw)
  To: Dikshita Agarwal, mchehab, hverkuil-cisco, ezequiel,
	boris.brezillon, ribalda, paul.kocialkowski, posciak,
	linux-media, stanimir.varbanov, linux-kernel
  Cc: linux-arm-msm, vgarodia, majja

Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
> LTR (Long Term Reference) frames are the frames that are encoded sometime in the past
> and stored in the DPB buffer list to be used as reference to encode future frames.
> One usage of LTR encoding is to reduce error propagation for video transmission
> in packet lossy networks.  For example, encoder may want to specify some key frames as
> LTR pictures and use them as reference frames for encoding. With extra protection
> selectively on these LTR frames or synchronization with the receiver of reception of
> the LTR frames during transmission, decoder can receive reference frames more reliably
> than other non-reference frames. As a result, transmission error can be effectively
> restricted within certain frames rather than propagated to future frames.
> 
> We are introducing below V4l2 Controls for this feature
> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>     a. This is used to query or configure the number of LTR frames.
>        This is a static control and is controlled by the client.
>     b. The LTR index varies from 0 to the max LTR-1.
>     c. If LTR Count is more than max supported LTR count (max LTR) by driver, it will be rejected.
>     d. Auto Marking : If LTR count is non zero,
>         1) first LTR count frames would be mark as LTR automatically after
>       	   every IDR frame (inclusive).
>         2) For multilayer encoding: first LTR count base layer reference frames starting after
>            every IDR frame (inclusive) in encoding order would be marked as LTR frames by the encoder.
>         3) Auto marking of LTR due to IDR should consider following conditions:
>             1. The frame is not already set to be marked as LTR.
>             2. The frame is part of the base layer in the hierarchical layer case.
>             3. The number of frames currently marked as LTR is less than the maximum LTR frame index plus 1.
>     e. Encoder needs to handle explicit Mark/Use command when encoder is still doing "auto" marking

Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some encoder
can select by themself long term references and even some encoders may
not let the user decide.

(not huge han of LTR acronyme, but that could be fine too, assuming you
add more _).

> 
> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>     a. This signals to mark the current frame as LTR frame. It is a dynamic control and also provide the LTR index to be used.
>     b. the LTR index provided by this control should never exceed the max LTR-1. Else it will be rejected.

The "current" frame seems a bit loose. Perhaps you wanted to use buffer
flags ? A bit like what we have to signal TOP/BOTTOM fields in
alternate interlacing.

> 
> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>     a. This specifies the LTR frame(s) to be used for encoding the current frame. This is a dynamic control.
>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N LSB bits of this field are valid,
>        where N is the maximum number of LTRs supported. All the other bits are invalid and should be rejected.
>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB corresponds to the LTR index max LTR-1.

Note, I haven't captured very well the userspace control flow, perhaps
this could be enhanced through writing some documentation.

As per all other generic encoder controls, we need to make sure it will
be usable and flexible enough for multiple HW blocks, as it can be
tedious to extend later otherwise. It is important that along with this
RFC you provide some comparisons with with other HW / SW APIs in order
to help justify the design decisions. I also think there should be 
link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.

regards,
Nicolas

> 
> Dikshita Agarwal (1):
>   media: v4l2-ctrls:  add control for ltr
> 
>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>  2 files changed, 10 insertions(+)
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-06-11 14:22 ` [RFC PATCH 0/1] Add LTR controls Nicolas Dufresne
@ 2020-06-12  9:11   ` Hans Verkuil
  2020-06-16 18:41     ` dikshita
  0 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2020-06-12  9:11 UTC (permalink / raw)
  To: Nicolas Dufresne, Dikshita Agarwal, mchehab, ezequiel,
	boris.brezillon, ribalda, paul.kocialkowski, posciak,
	linux-media, stanimir.varbanov, linux-kernel
  Cc: linux-arm-msm, vgarodia, majja

Hi Dikshita, Nicolas,

On 11/06/2020 16:22, Nicolas Dufresne wrote:
> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>> LTR (Long Term Reference) frames are the frames that are encoded sometime in the past
>> and stored in the DPB buffer list to be used as reference to encode future frames.
>> One usage of LTR encoding is to reduce error propagation for video transmission
>> in packet lossy networks.  For example, encoder may want to specify some key frames as
>> LTR pictures and use them as reference frames for encoding. With extra protection
>> selectively on these LTR frames or synchronization with the receiver of reception of
>> the LTR frames during transmission, decoder can receive reference frames more reliably
>> than other non-reference frames. As a result, transmission error can be effectively
>> restricted within certain frames rather than propagated to future frames.
>>
>> We are introducing below V4l2 Controls for this feature
>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>     a. This is used to query or configure the number of LTR frames.
>>        This is a static control and is controlled by the client.
>>     b. The LTR index varies from 0 to the max LTR-1.
>>     c. If LTR Count is more than max supported LTR count (max LTR) by driver, it will be rejected.
>>     d. Auto Marking : If LTR count is non zero,
>>         1) first LTR count frames would be mark as LTR automatically after
>>       	   every IDR frame (inclusive).
>>         2) For multilayer encoding: first LTR count base layer reference frames starting after
>>            every IDR frame (inclusive) in encoding order would be marked as LTR frames by the encoder.
>>         3) Auto marking of LTR due to IDR should consider following conditions:
>>             1. The frame is not already set to be marked as LTR.
>>             2. The frame is part of the base layer in the hierarchical layer case.
>>             3. The number of frames currently marked as LTR is less than the maximum LTR frame index plus 1.
>>     e. Encoder needs to handle explicit Mark/Use command when encoder is still doing "auto" marking

I don't follow this, quite possibly due to lack of experience with encoders.

I kind of would expect to see two modes: either automatic where encoders can
mark up to LTR_COUNT frames as long term reference, and userspace just sets
LTR_COUNT and doesn't have to do anything else.

Or it is manual mode where userspace explicitly marks long term reference
frames.

From the proposal above it looks like you can mix auto and manual modes.

BTW, how do you 'unmark' long term reference frames?

This feature is for stateful encoders, right?

> 
> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some encoder
> can select by themself long term references and even some encoders may
> not let the user decide.
> 
> (not huge han of LTR acronyme, but that could be fine too, assuming you
> add more _).
> 
>>
>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>     a. This signals to mark the current frame as LTR frame. It is a dynamic control and also provide the LTR index to be used.
>>     b. the LTR index provided by this control should never exceed the max LTR-1. Else it will be rejected.
> 
> The "current" frame seems a bit loose. Perhaps you wanted to use buffer
> flags ? A bit like what we have to signal TOP/BOTTOM fields in
> alternate interlacing.

I was thinking the same thing. Using a control for this doesn't seem right.

> 
>>
>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>     a. This specifies the LTR frame(s) to be used for encoding the current frame. This is a dynamic control.
>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N LSB bits of this field are valid,
>>        where N is the maximum number of LTRs supported. All the other bits are invalid and should be rejected.
>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB corresponds to the LTR index max LTR-1.

How would userspace know this? Esp. with auto marking since userspace would have
to predict how auto marking works (if I understand this correctly).

For which HW encoder is this meant?

> 
> Note, I haven't captured very well the userspace control flow, perhaps
> this could be enhanced through writing some documentation.
> 
> As per all other generic encoder controls, we need to make sure it will
> be usable and flexible enough for multiple HW blocks, as it can be
> tedious to extend later otherwise. It is important that along with this
> RFC you provide some comparisons with with other HW / SW APIs in order
> to help justify the design decisions. I also think there should be 
> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.

I agree with Nicolas.

Regards,

	Hans

> 
> regards,
> Nicolas
> 
>>
>> Dikshita Agarwal (1):
>>   media: v4l2-ctrls:  add control for ltr
>>
>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>  2 files changed, 10 insertions(+)
>>
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-06-12  9:11   ` Hans Verkuil
@ 2020-06-16 18:41     ` dikshita
  2020-07-14  7:21       ` dikshita
  2020-07-16  8:55       ` Hans Verkuil
  0 siblings, 2 replies; 11+ messages in thread
From: dikshita @ 2020-06-16 18:41 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

Hi Hans, Nicolas,

Thanks for your comments.

On 2020-06-12 14:41, Hans Verkuil wrote:
> Hi Dikshita, Nicolas,
> 
> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>> LTR (Long Term Reference) frames are the frames that are encoded 
>>> sometime in the past
>>> and stored in the DPB buffer list to be used as reference to encode 
>>> future frames.
>>> One usage of LTR encoding is to reduce error propagation for video 
>>> transmission
>>> in packet lossy networks.  For example, encoder may want to specify 
>>> some key frames as
>>> LTR pictures and use them as reference frames for encoding. With 
>>> extra protection
>>> selectively on these LTR frames or synchronization with the receiver 
>>> of reception of
>>> the LTR frames during transmission, decoder can receive reference 
>>> frames more reliably
>>> than other non-reference frames. As a result, transmission error can 
>>> be effectively
>>> restricted within certain frames rather than propagated to future 
>>> frames.
>>> 
>>> We are introducing below V4l2 Controls for this feature
>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>     a. This is used to query or configure the number of LTR frames.
>>>        This is a static control and is controlled by the client.
>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>     c. If LTR Count is more than max supported LTR count (max LTR) by 
>>> driver, it will be rejected.
>>>     d. Auto Marking : If LTR count is non zero,
>>>         1) first LTR count frames would be mark as LTR automatically 
>>> after
>>>       	   every IDR frame (inclusive).
>>>         2) For multilayer encoding: first LTR count base layer 
>>> reference frames starting after
>>>            every IDR frame (inclusive) in encoding order would be 
>>> marked as LTR frames by the encoder.
>>>         3) Auto marking of LTR due to IDR should consider following 
>>> conditions:
>>>             1. The frame is not already set to be marked as LTR.
>>>             2. The frame is part of the base layer in the 
>>> hierarchical layer case.
>>>             3. The number of frames currently marked as LTR is less 
>>> than the maximum LTR frame index plus 1.
>>>     e. Encoder needs to handle explicit Mark/Use command when encoder 
>>> is still doing "auto" marking
> 
> I don't follow this, quite possibly due to lack of experience with 
> encoders.
> 
> I kind of would expect to see two modes: either automatic where 
> encoders can
> mark up to LTR_COUNT frames as long term reference, and userspace just 
> sets
> LTR_COUNT and doesn't have to do anything else.
> 
> Or it is manual mode where userspace explicitly marks long term 
> reference
> frames.
> 
> From the proposal above it looks like you can mix auto and manual 
> modes.
> 
> BTW, how do you 'unmark' long term reference frames?
> 
> This feature is for stateful encoders, right?
> 
>> 
>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some encoder
>> can select by themself long term references and even some encoders may
>> not let the user decide.
>> 
>> (not huge han of LTR acronyme, but that could be fine too, assuming 
>> you
>> add more _).
>> 

Userspace sets LTR count which signifies the number of LTR frames 
encoder needs to generate or keep.
The encoder has to build-up its internal buffer reference list (aka DBP 
list or recon buffer list).
In order to achieve that encoder will fill It's LTR (long term 
references) list and STR (short term references) list
by auto marking n frames as LTR frames(n is equal to LTR count) based on 
auto-marking dictated by the encoder spec.
The client then can replace those automatically marked frames with new 
frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
encoder to refer the newly marked frame for encoding the next frame 
using V4L2_CID_MPEG_VIDEO_USELTRFRAME.

>>> 
>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>     a. This signals to mark the current frame as LTR frame. It is a 
>>> dynamic control and also provide the LTR index to be used.
>>>     b. the LTR index provided by this control should never exceed the 
>>> max LTR-1. Else it will be rejected.
>> 
>> The "current" frame seems a bit loose. Perhaps you wanted to use 
>> buffer
>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>> alternate interlacing.
> 
> I was thinking the same thing. Using a control for this doesn't seem 
> right.
> 

the client sets this to replace automatically marked frames by the 
encoder with a particular frame.
this provides an index that ranges from 0 to LTR count-1 and then the 
particular frame will be marked with that index.
this can be achieved through request by associating this control with a 
specific buffer to make it synchronized.

>> 
>>> 
>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>     a. This specifies the LTR frame(s) to be used for encoding the 
>>> current frame. This is a dynamic control.
>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N 
>>> LSB bits of this field are valid,
>>>        where N is the maximum number of LTRs supported. All the other 
>>> bits are invalid and should be rejected.
>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB 
>>> corresponds to the LTR index max LTR-1.
> 
> How would userspace know this? Esp. with auto marking since userspace 
> would have
> to predict how auto marking works (if I understand this correctly).
> 

Client sets LTR count which tells about the number of LTR frames 
automatically marked by the encoder.
so client can use LTR index (0 to LTR count -1) to ask encoder to refer 
any particular
frame (marked automatically by driver or marked by client with 
V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next 
frame.

> For which HW encoder is this meant?
> 
This is primarily meant for H264 and HEVC.

Thanks,
Dikshita

>> 
>> Note, I haven't captured very well the userspace control flow, perhaps
>> this could be enhanced through writing some documentation.
>> 
>> As per all other generic encoder controls, we need to make sure it 
>> will
>> be usable and flexible enough for multiple HW blocks, as it can be
>> tedious to extend later otherwise. It is important that along with 
>> this
>> RFC you provide some comparisons with with other HW / SW APIs in order
>> to help justify the design decisions. I also think there should be
>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
> 
> I agree with Nicolas.
> 
> Regards,
> 
> 	Hans
> 
>> 
>> regards,
>> Nicolas
>> 
>>> 
>>> Dikshita Agarwal (1):
>>>   media: v4l2-ctrls:  add control for ltr
>>> 
>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>  2 files changed, 10 insertions(+)
>>> 
>> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-06-16 18:41     ` dikshita
@ 2020-07-14  7:21       ` dikshita
  2020-07-16  8:55       ` Hans Verkuil
  1 sibling, 0 replies; 11+ messages in thread
From: dikshita @ 2020-07-14  7:21 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

Hi,

A gentle reminder for the review.

Thanks,
Dikshita

On 2020-06-17 00:11, dikshita@codeaurora.org wrote:
> Hi Hans, Nicolas,
> 
> Thanks for your comments.
> 
> On 2020-06-12 14:41, Hans Verkuil wrote:
>> Hi Dikshita, Nicolas,
>> 
>> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>>> LTR (Long Term Reference) frames are the frames that are encoded 
>>>> sometime in the past
>>>> and stored in the DPB buffer list to be used as reference to encode 
>>>> future frames.
>>>> One usage of LTR encoding is to reduce error propagation for video 
>>>> transmission
>>>> in packet lossy networks.  For example, encoder may want to specify 
>>>> some key frames as
>>>> LTR pictures and use them as reference frames for encoding. With 
>>>> extra protection
>>>> selectively on these LTR frames or synchronization with the receiver 
>>>> of reception of
>>>> the LTR frames during transmission, decoder can receive reference 
>>>> frames more reliably
>>>> than other non-reference frames. As a result, transmission error can 
>>>> be effectively
>>>> restricted within certain frames rather than propagated to future 
>>>> frames.
>>>> 
>>>> We are introducing below V4l2 Controls for this feature
>>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>>     a. This is used to query or configure the number of LTR frames.
>>>>        This is a static control and is controlled by the client.
>>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>>     c. If LTR Count is more than max supported LTR count (max LTR) 
>>>> by driver, it will be rejected.
>>>>     d. Auto Marking : If LTR count is non zero,
>>>>         1) first LTR count frames would be mark as LTR automatically 
>>>> after
>>>>       	   every IDR frame (inclusive).
>>>>         2) For multilayer encoding: first LTR count base layer 
>>>> reference frames starting after
>>>>            every IDR frame (inclusive) in encoding order would be 
>>>> marked as LTR frames by the encoder.
>>>>         3) Auto marking of LTR due to IDR should consider following 
>>>> conditions:
>>>>             1. The frame is not already set to be marked as LTR.
>>>>             2. The frame is part of the base layer in the 
>>>> hierarchical layer case.
>>>>             3. The number of frames currently marked as LTR is less 
>>>> than the maximum LTR frame index plus 1.
>>>>     e. Encoder needs to handle explicit Mark/Use command when 
>>>> encoder is still doing "auto" marking
>> 
>> I don't follow this, quite possibly due to lack of experience with 
>> encoders.
>> 
>> I kind of would expect to see two modes: either automatic where 
>> encoders can
>> mark up to LTR_COUNT frames as long term reference, and userspace just 
>> sets
>> LTR_COUNT and doesn't have to do anything else.
>> 
>> Or it is manual mode where userspace explicitly marks long term 
>> reference
>> frames.
>> 
>> From the proposal above it looks like you can mix auto and manual 
>> modes.
>> 
>> BTW, how do you 'unmark' long term reference frames?
>> 
>> This feature is for stateful encoders, right?
>> 
>>> 
>>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some 
>>> encoder
>>> can select by themself long term references and even some encoders 
>>> may
>>> not let the user decide.
>>> 
>>> (not huge han of LTR acronyme, but that could be fine too, assuming 
>>> you
>>> add more _).
>>> 
> 
> Userspace sets LTR count which signifies the number of LTR frames
> encoder needs to generate or keep.
> The encoder has to build-up its internal buffer reference list (aka
> DBP list or recon buffer list).
> In order to achieve that encoder will fill It's LTR (long term
> references) list and STR (short term references) list
> by auto marking n frames as LTR frames(n is equal to LTR count) based
> on auto-marking dictated by the encoder spec.
> The client then can replace those automatically marked frames with new
> frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
> encoder to refer the newly marked frame for encoding the next frame
> using V4L2_CID_MPEG_VIDEO_USELTRFRAME.
> 
>>>> 
>>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>>     a. This signals to mark the current frame as LTR frame. It is a 
>>>> dynamic control and also provide the LTR index to be used.
>>>>     b. the LTR index provided by this control should never exceed 
>>>> the max LTR-1. Else it will be rejected.
>>> 
>>> The "current" frame seems a bit loose. Perhaps you wanted to use 
>>> buffer
>>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>>> alternate interlacing.
>> 
>> I was thinking the same thing. Using a control for this doesn't seem 
>> right.
>> 
> 
> the client sets this to replace automatically marked frames by the
> encoder with a particular frame.
> this provides an index that ranges from 0 to LTR count-1 and then the
> particular frame will be marked with that index.
> this can be achieved through request by associating this control with
> a specific buffer to make it synchronized.
> 
>>> 
>>>> 
>>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>>     a. This specifies the LTR frame(s) to be used for encoding the 
>>>> current frame. This is a dynamic control.
>>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N 
>>>> LSB bits of this field are valid,
>>>>        where N is the maximum number of LTRs supported. All the 
>>>> other bits are invalid and should be rejected.
>>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB 
>>>> corresponds to the LTR index max LTR-1.
>> 
>> How would userspace know this? Esp. with auto marking since userspace 
>> would have
>> to predict how auto marking works (if I understand this correctly).
>> 
> 
> Client sets LTR count which tells about the number of LTR frames
> automatically marked by the encoder.
> so client can use LTR index (0 to LTR count -1) to ask encoder to
> refer any particular
> frame (marked automatically by driver or marked by client with
> V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next
> frame.
> 
>> For which HW encoder is this meant?
>> 
> This is primarily meant for H264 and HEVC.
> 
> Thanks,
> Dikshita
> 
>>> 
>>> Note, I haven't captured very well the userspace control flow, 
>>> perhaps
>>> this could be enhanced through writing some documentation.
>>> 
>>> As per all other generic encoder controls, we need to make sure it 
>>> will
>>> be usable and flexible enough for multiple HW blocks, as it can be
>>> tedious to extend later otherwise. It is important that along with 
>>> this
>>> RFC you provide some comparisons with with other HW / SW APIs in 
>>> order
>>> to help justify the design decisions. I also think there should be
>>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
>> 
>> I agree with Nicolas.
>> 
>> Regards,
>> 
>> 	Hans
>> 
>>> 
>>> regards,
>>> Nicolas
>>> 
>>>> 
>>>> Dikshita Agarwal (1):
>>>>   media: v4l2-ctrls:  add control for ltr
>>>> 
>>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>>  2 files changed, 10 insertions(+)
>>>> 
>>> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-06-16 18:41     ` dikshita
  2020-07-14  7:21       ` dikshita
@ 2020-07-16  8:55       ` Hans Verkuil
  2020-07-20 14:33         ` dikshita
  1 sibling, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2020-07-16  8:55 UTC (permalink / raw)
  To: dikshita
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

On 16/06/2020 20:41, dikshita@codeaurora.org wrote:
> Hi Hans, Nicolas,
> 
> Thanks for your comments.
> 
> On 2020-06-12 14:41, Hans Verkuil wrote:
>> Hi Dikshita, Nicolas,
>>
>> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>>> LTR (Long Term Reference) frames are the frames that are encoded 
>>>> sometime in the past
>>>> and stored in the DPB buffer list to be used as reference to encode 
>>>> future frames.
>>>> One usage of LTR encoding is to reduce error propagation for video 
>>>> transmission
>>>> in packet lossy networks.  For example, encoder may want to specify 
>>>> some key frames as
>>>> LTR pictures and use them as reference frames for encoding. With 
>>>> extra protection
>>>> selectively on these LTR frames or synchronization with the receiver 
>>>> of reception of
>>>> the LTR frames during transmission, decoder can receive reference 
>>>> frames more reliably
>>>> than other non-reference frames. As a result, transmission error can 
>>>> be effectively
>>>> restricted within certain frames rather than propagated to future 
>>>> frames.
>>>>
>>>> We are introducing below V4l2 Controls for this feature
>>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>>     a. This is used to query or configure the number of LTR frames.
>>>>        This is a static control and is controlled by the client.
>>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>>     c. If LTR Count is more than max supported LTR count (max LTR) by 
>>>> driver, it will be rejected.
>>>>     d. Auto Marking : If LTR count is non zero,
>>>>         1) first LTR count frames would be mark as LTR automatically 
>>>> after
>>>>       	   every IDR frame (inclusive).
>>>>         2) For multilayer encoding: first LTR count base layer 
>>>> reference frames starting after
>>>>            every IDR frame (inclusive) in encoding order would be 
>>>> marked as LTR frames by the encoder.
>>>>         3) Auto marking of LTR due to IDR should consider following 
>>>> conditions:
>>>>             1. The frame is not already set to be marked as LTR.
>>>>             2. The frame is part of the base layer in the 
>>>> hierarchical layer case.
>>>>             3. The number of frames currently marked as LTR is less 
>>>> than the maximum LTR frame index plus 1.
>>>>     e. Encoder needs to handle explicit Mark/Use command when encoder 
>>>> is still doing "auto" marking
>>
>> I don't follow this, quite possibly due to lack of experience with 
>> encoders.
>>
>> I kind of would expect to see two modes: either automatic where 
>> encoders can
>> mark up to LTR_COUNT frames as long term reference, and userspace just 
>> sets
>> LTR_COUNT and doesn't have to do anything else.
>>
>> Or it is manual mode where userspace explicitly marks long term 
>> reference
>> frames.
>>
>> From the proposal above it looks like you can mix auto and manual 
>> modes.
>>
>> BTW, how do you 'unmark' long term reference frames?
>>
>> This feature is for stateful encoders, right?
>>
>>>
>>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some encoder
>>> can select by themself long term references and even some encoders may
>>> not let the user decide.
>>>
>>> (not huge han of LTR acronyme, but that could be fine too, assuming 
>>> you
>>> add more _).
>>>
> 
> Userspace sets LTR count which signifies the number of LTR frames 
> encoder needs to generate or keep.
> The encoder has to build-up its internal buffer reference list (aka DBP 
> list or recon buffer list).
> In order to achieve that encoder will fill It's LTR (long term 
> references) list and STR (short term references) list
> by auto marking n frames as LTR frames(n is equal to LTR count) based on 
> auto-marking dictated by the encoder spec.
> The client then can replace those automatically marked frames with new 
> frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
> encoder to refer the newly marked frame for encoding the next frame 
> using V4L2_CID_MPEG_VIDEO_USELTRFRAME.
> 
>>>>
>>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>>     a. This signals to mark the current frame as LTR frame. It is a 
>>>> dynamic control and also provide the LTR index to be used.
>>>>     b. the LTR index provided by this control should never exceed the 
>>>> max LTR-1. Else it will be rejected.
>>>
>>> The "current" frame seems a bit loose. Perhaps you wanted to use 
>>> buffer
>>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>>> alternate interlacing.
>>
>> I was thinking the same thing. Using a control for this doesn't seem 
>> right.
>>
> 
> the client sets this to replace automatically marked frames by the 
> encoder with a particular frame.
> this provides an index that ranges from 0 to LTR count-1 and then the 
> particular frame will be marked with that index.
> this can be achieved through request by associating this control with a 
> specific buffer to make it synchronized.
> 
>>>
>>>>
>>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>>     a. This specifies the LTR frame(s) to be used for encoding the 
>>>> current frame. This is a dynamic control.
>>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N 
>>>> LSB bits of this field are valid,
>>>>        where N is the maximum number of LTRs supported. All the other 
>>>> bits are invalid and should be rejected.
>>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB 
>>>> corresponds to the LTR index max LTR-1.
>>
>> How would userspace know this? Esp. with auto marking since userspace 
>> would have
>> to predict how auto marking works (if I understand this correctly).
>>
> 
> Client sets LTR count which tells about the number of LTR frames 
> automatically marked by the encoder.
> so client can use LTR index (0 to LTR count -1) to ask encoder to refer 
> any particular
> frame (marked automatically by driver or marked by client with 
> V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next 
> frame.
> 
>> For which HW encoder is this meant?
>>
> This is primarily meant for H264 and HEVC.

The venus encoder?

Some more questions:

1) How many LTR frames do h.264 and hevc allow?
2) Given N LTR frames, is there a ordering of those frames? E.g.
   the LTR frame with index 0 is processed/used differently from
   LTR frame with index 1? Or are they all equal in that it is just a pool
   of LTR frames that the encoder can use as it wishes?

Regards,

	Hans

> 
> Thanks,
> Dikshita
> 
>>>
>>> Note, I haven't captured very well the userspace control flow, perhaps
>>> this could be enhanced through writing some documentation.
>>>
>>> As per all other generic encoder controls, we need to make sure it 
>>> will
>>> be usable and flexible enough for multiple HW blocks, as it can be
>>> tedious to extend later otherwise. It is important that along with 
>>> this
>>> RFC you provide some comparisons with with other HW / SW APIs in order
>>> to help justify the design decisions. I also think there should be
>>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
>>
>> I agree with Nicolas.
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> regards,
>>> Nicolas
>>>
>>>>
>>>> Dikshita Agarwal (1):
>>>>   media: v4l2-ctrls:  add control for ltr
>>>>
>>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>>  2 files changed, 10 insertions(+)
>>>>
>>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-07-16  8:55       ` Hans Verkuil
@ 2020-07-20 14:33         ` dikshita
  2020-07-22 14:26           ` Hans Verkuil
  0 siblings, 1 reply; 11+ messages in thread
From: dikshita @ 2020-07-20 14:33 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

On 2020-07-16 14:25, Hans Verkuil wrote:
> On 16/06/2020 20:41, dikshita@codeaurora.org wrote:
>> Hi Hans, Nicolas,
>> 
>> Thanks for your comments.
>> 
>> On 2020-06-12 14:41, Hans Verkuil wrote:
>>> Hi Dikshita, Nicolas,
>>> 
>>> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>>>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>>>> LTR (Long Term Reference) frames are the frames that are encoded
>>>>> sometime in the past
>>>>> and stored in the DPB buffer list to be used as reference to encode
>>>>> future frames.
>>>>> One usage of LTR encoding is to reduce error propagation for video
>>>>> transmission
>>>>> in packet lossy networks.  For example, encoder may want to specify
>>>>> some key frames as
>>>>> LTR pictures and use them as reference frames for encoding. With
>>>>> extra protection
>>>>> selectively on these LTR frames or synchronization with the 
>>>>> receiver
>>>>> of reception of
>>>>> the LTR frames during transmission, decoder can receive reference
>>>>> frames more reliably
>>>>> than other non-reference frames. As a result, transmission error 
>>>>> can
>>>>> be effectively
>>>>> restricted within certain frames rather than propagated to future
>>>>> frames.
>>>>> 
>>>>> We are introducing below V4l2 Controls for this feature
>>>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>>>     a. This is used to query or configure the number of LTR frames.
>>>>>        This is a static control and is controlled by the client.
>>>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>>>     c. If LTR Count is more than max supported LTR count (max LTR) 
>>>>> by
>>>>> driver, it will be rejected.
>>>>>     d. Auto Marking : If LTR count is non zero,
>>>>>         1) first LTR count frames would be mark as LTR 
>>>>> automatically
>>>>> after
>>>>>       	   every IDR frame (inclusive).
>>>>>         2) For multilayer encoding: first LTR count base layer
>>>>> reference frames starting after
>>>>>            every IDR frame (inclusive) in encoding order would be
>>>>> marked as LTR frames by the encoder.
>>>>>         3) Auto marking of LTR due to IDR should consider following
>>>>> conditions:
>>>>>             1. The frame is not already set to be marked as LTR.
>>>>>             2. The frame is part of the base layer in the
>>>>> hierarchical layer case.
>>>>>             3. The number of frames currently marked as LTR is less
>>>>> than the maximum LTR frame index plus 1.
>>>>>     e. Encoder needs to handle explicit Mark/Use command when 
>>>>> encoder
>>>>> is still doing "auto" marking
>>> 
>>> I don't follow this, quite possibly due to lack of experience with
>>> encoders.
>>> 
>>> I kind of would expect to see two modes: either automatic where
>>> encoders can
>>> mark up to LTR_COUNT frames as long term reference, and userspace 
>>> just
>>> sets
>>> LTR_COUNT and doesn't have to do anything else.
>>> 
>>> Or it is manual mode where userspace explicitly marks long term
>>> reference
>>> frames.
>>> 
>>> From the proposal above it looks like you can mix auto and manual
>>> modes.
>>> 
>>> BTW, how do you 'unmark' long term reference frames?
>>> 
>>> This feature is for stateful encoders, right?
>>> 
>>>> 
>>>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some 
>>>> encoder
>>>> can select by themself long term references and even some encoders 
>>>> may
>>>> not let the user decide.
>>>> 
>>>> (not huge han of LTR acronyme, but that could be fine too, assuming
>>>> you
>>>> add more _).
>>>> 
>> 
>> Userspace sets LTR count which signifies the number of LTR frames
>> encoder needs to generate or keep.
>> The encoder has to build-up its internal buffer reference list (aka 
>> DBP
>> list or recon buffer list).
>> In order to achieve that encoder will fill It's LTR (long term
>> references) list and STR (short term references) list
>> by auto marking n frames as LTR frames(n is equal to LTR count) based 
>> on
>> auto-marking dictated by the encoder spec.
>> The client then can replace those automatically marked frames with new
>> frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
>> encoder to refer the newly marked frame for encoding the next frame
>> using V4L2_CID_MPEG_VIDEO_USELTRFRAME.
>> 
>>>>> 
>>>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>>>     a. This signals to mark the current frame as LTR frame. It is a
>>>>> dynamic control and also provide the LTR index to be used.
>>>>>     b. the LTR index provided by this control should never exceed 
>>>>> the
>>>>> max LTR-1. Else it will be rejected.
>>>> 
>>>> The "current" frame seems a bit loose. Perhaps you wanted to use
>>>> buffer
>>>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>>>> alternate interlacing.
>>> 
>>> I was thinking the same thing. Using a control for this doesn't seem
>>> right.
>>> 
>> 
>> the client sets this to replace automatically marked frames by the
>> encoder with a particular frame.
>> this provides an index that ranges from 0 to LTR count-1 and then the
>> particular frame will be marked with that index.
>> this can be achieved through request by associating this control with 
>> a
>> specific buffer to make it synchronized.
>> 
>>>> 
>>>>> 
>>>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>>>     a. This specifies the LTR frame(s) to be used for encoding the
>>>>> current frame. This is a dynamic control.
>>>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N
>>>>> LSB bits of this field are valid,
>>>>>        where N is the maximum number of LTRs supported. All the 
>>>>> other
>>>>> bits are invalid and should be rejected.
>>>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB
>>>>> corresponds to the LTR index max LTR-1.
>>> 
>>> How would userspace know this? Esp. with auto marking since userspace
>>> would have
>>> to predict how auto marking works (if I understand this correctly).
>>> 
>> 
>> Client sets LTR count which tells about the number of LTR frames
>> automatically marked by the encoder.
>> so client can use LTR index (0 to LTR count -1) to ask encoder to 
>> refer
>> any particular
>> frame (marked automatically by driver or marked by client with
>> V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next
>> frame.
>> 
>>> For which HW encoder is this meant?
>>> 
>> This is primarily meant for H264 and HEVC.
> 
> The venus encoder?
yes
> 
> Some more questions:
> 
> 1) How many LTR frames do h.264 and hevc allow?
According to spec, MAX LTR allowed by H.264 is 15 and HEVC allows max 32 
LTR frames.
> 2) Given N LTR frames, is there a ordering of those frames? E.g.
>    the LTR frame with index 0 is processed/used differently from
>    LTR frame with index 1? Or are they all equal in that it is just a 
> pool
>    of LTR frames that the encoder can use as it wishes?
they are different frames marked to be used as LTR and stored in 
available indices.
Userspace notifies encoder which LTR frame index to use via USE LTR 
control.

Thanks,
Dikshita
> 
> Regards,
> 
> 	Hans
> 
>> 
>> Thanks,
>> Dikshita
>> 
>>>> 
>>>> Note, I haven't captured very well the userspace control flow, 
>>>> perhaps
>>>> this could be enhanced through writing some documentation.
>>>> 
>>>> As per all other generic encoder controls, we need to make sure it
>>>> will
>>>> be usable and flexible enough for multiple HW blocks, as it can be
>>>> tedious to extend later otherwise. It is important that along with
>>>> this
>>>> RFC you provide some comparisons with with other HW / SW APIs in 
>>>> order
>>>> to help justify the design decisions. I also think there should be
>>>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
>>> 
>>> I agree with Nicolas.
>>> 
>>> Regards,
>>> 
>>> 	Hans
>>> 
>>>> 
>>>> regards,
>>>> Nicolas
>>>> 
>>>>> 
>>>>> Dikshita Agarwal (1):
>>>>>   media: v4l2-ctrls:  add control for ltr
>>>>> 
>>>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>>>  2 files changed, 10 insertions(+)
>>>>> 
>>>> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-07-20 14:33         ` dikshita
@ 2020-07-22 14:26           ` Hans Verkuil
  2020-08-06 11:05             ` dikshita
  0 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2020-07-22 14:26 UTC (permalink / raw)
  To: dikshita
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

On 20/07/2020 16:33, dikshita@codeaurora.org wrote:
> On 2020-07-16 14:25, Hans Verkuil wrote:
>> On 16/06/2020 20:41, dikshita@codeaurora.org wrote:
>>> Hi Hans, Nicolas,
>>>
>>> Thanks for your comments.
>>>
>>> On 2020-06-12 14:41, Hans Verkuil wrote:
>>>> Hi Dikshita, Nicolas,
>>>>
>>>> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>>>>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>>>>> LTR (Long Term Reference) frames are the frames that are encoded
>>>>>> sometime in the past
>>>>>> and stored in the DPB buffer list to be used as reference to encode
>>>>>> future frames.
>>>>>> One usage of LTR encoding is to reduce error propagation for video
>>>>>> transmission
>>>>>> in packet lossy networks.  For example, encoder may want to specify
>>>>>> some key frames as
>>>>>> LTR pictures and use them as reference frames for encoding. With
>>>>>> extra protection
>>>>>> selectively on these LTR frames or synchronization with the receiver
>>>>>> of reception of
>>>>>> the LTR frames during transmission, decoder can receive reference
>>>>>> frames more reliably
>>>>>> than other non-reference frames. As a result, transmission error can
>>>>>> be effectively
>>>>>> restricted within certain frames rather than propagated to future
>>>>>> frames.
>>>>>>
>>>>>> We are introducing below V4l2 Controls for this feature
>>>>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>>>>     a. This is used to query or configure the number of LTR frames.
>>>>>>        This is a static control and is controlled by the client.
>>>>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>>>>     c. If LTR Count is more than max supported LTR count (max LTR) by
>>>>>> driver, it will be rejected.
>>>>>>     d. Auto Marking : If LTR count is non zero,
>>>>>>         1) first LTR count frames would be mark as LTR automatically
>>>>>> after
>>>>>>              every IDR frame (inclusive).
>>>>>>         2) For multilayer encoding: first LTR count base layer
>>>>>> reference frames starting after
>>>>>>            every IDR frame (inclusive) in encoding order would be
>>>>>> marked as LTR frames by the encoder.
>>>>>>         3) Auto marking of LTR due to IDR should consider following
>>>>>> conditions:
>>>>>>             1. The frame is not already set to be marked as LTR.
>>>>>>             2. The frame is part of the base layer in the
>>>>>> hierarchical layer case.
>>>>>>             3. The number of frames currently marked as LTR is less
>>>>>> than the maximum LTR frame index plus 1.
>>>>>>     e. Encoder needs to handle explicit Mark/Use command when encoder
>>>>>> is still doing "auto" marking
>>>>
>>>> I don't follow this, quite possibly due to lack of experience with
>>>> encoders.
>>>>
>>>> I kind of would expect to see two modes: either automatic where
>>>> encoders can
>>>> mark up to LTR_COUNT frames as long term reference, and userspace just
>>>> sets
>>>> LTR_COUNT and doesn't have to do anything else.
>>>>
>>>> Or it is manual mode where userspace explicitly marks long term
>>>> reference
>>>> frames.
>>>>
>>>> From the proposal above it looks like you can mix auto and manual
>>>> modes.
>>>>
>>>> BTW, how do you 'unmark' long term reference frames?
>>>>
>>>> This feature is for stateful encoders, right?
>>>>
>>>>>
>>>>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some encoder
>>>>> can select by themself long term references and even some encoders may
>>>>> not let the user decide.
>>>>>
>>>>> (not huge han of LTR acronyme, but that could be fine too, assuming
>>>>> you
>>>>> add more _).
>>>>>
>>>
>>> Userspace sets LTR count which signifies the number of LTR frames
>>> encoder needs to generate or keep.
>>> The encoder has to build-up its internal buffer reference list (aka DBP
>>> list or recon buffer list).
>>> In order to achieve that encoder will fill It's LTR (long term
>>> references) list and STR (short term references) list
>>> by auto marking n frames as LTR frames(n is equal to LTR count) based on
>>> auto-marking dictated by the encoder spec.
>>> The client then can replace those automatically marked frames with new
>>> frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
>>> encoder to refer the newly marked frame for encoding the next frame
>>> using V4L2_CID_MPEG_VIDEO_USELTRFRAME.
>>>
>>>>>>
>>>>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>>>>     a. This signals to mark the current frame as LTR frame. It is a
>>>>>> dynamic control and also provide the LTR index to be used.
>>>>>>     b. the LTR index provided by this control should never exceed the
>>>>>> max LTR-1. Else it will be rejected.
>>>>>
>>>>> The "current" frame seems a bit loose. Perhaps you wanted to use
>>>>> buffer
>>>>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>>>>> alternate interlacing.
>>>>
>>>> I was thinking the same thing. Using a control for this doesn't seem
>>>> right.
>>>>
>>>
>>> the client sets this to replace automatically marked frames by the
>>> encoder with a particular frame.
>>> this provides an index that ranges from 0 to LTR count-1 and then the
>>> particular frame will be marked with that index.
>>> this can be achieved through request by associating this control with a
>>> specific buffer to make it synchronized.
>>>
>>>>>
>>>>>>
>>>>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>>>>     a. This specifies the LTR frame(s) to be used for encoding the
>>>>>> current frame. This is a dynamic control.
>>>>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N
>>>>>> LSB bits of this field are valid,
>>>>>>        where N is the maximum number of LTRs supported. All the other
>>>>>> bits are invalid and should be rejected.
>>>>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB
>>>>>> corresponds to the LTR index max LTR-1.
>>>>
>>>> How would userspace know this? Esp. with auto marking since userspace
>>>> would have
>>>> to predict how auto marking works (if I understand this correctly).
>>>>
>>>
>>> Client sets LTR count which tells about the number of LTR frames
>>> automatically marked by the encoder.
>>> so client can use LTR index (0 to LTR count -1) to ask encoder to refer
>>> any particular
>>> frame (marked automatically by driver or marked by client with
>>> V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next
>>> frame.
>>>
>>>> For which HW encoder is this meant?
>>>>
>>> This is primarily meant for H264 and HEVC.
>>
>> The venus encoder?
> yes
>>
>> Some more questions:
>>
>> 1) How many LTR frames do h.264 and hevc allow?
> According to spec, MAX LTR allowed by H.264 is 15 and HEVC allows max 32 LTR frames.
>> 2) Given N LTR frames, is there a ordering of those frames? E.g.
>>    the LTR frame with index 0 is processed/used differently from
>>    LTR frame with index 1? Or are they all equal in that it is just a pool
>>    of LTR frames that the encoder can use as it wishes?
> they are different frames marked to be used as LTR and stored in available indices.
> Userspace notifies encoder which LTR frame index to use via USE LTR control.

One more question: if a frame is marked as a LTR frame, does that mean that userspace
can't reuse the buffer containing that frame? I assume that's the case.

One thing that I don't like about this API is that it introduces the LTR indices.
Would it be possible to instead use buffer indices? (i.e. the v4l2_buffer index field).
Then you can use a buffer flag to indicate that a buffer should be an LTR buffer,
or (for automarking) the driver can mark a buffer as an LTR buffer by setting this flag
before returning the buffer to userspace.

You would still need a V4L2_CID_MPEG_VIDEO_USELTRFRAME, but that can be a bitmask
using the buffer index.

This would avoid introducing a second method of referring to buffers (LTR index),
and I think that V4L2_CID_MPEG_VIDEO_MARKLTRFRAME can be dropped.

Regards,

	Hans

> 
> Thanks,
> Dikshita
>>
>> Regards,
>>
>>     Hans
>>
>>>
>>> Thanks,
>>> Dikshita
>>>
>>>>>
>>>>> Note, I haven't captured very well the userspace control flow, perhaps
>>>>> this could be enhanced through writing some documentation.
>>>>>
>>>>> As per all other generic encoder controls, we need to make sure it
>>>>> will
>>>>> be usable and flexible enough for multiple HW blocks, as it can be
>>>>> tedious to extend later otherwise. It is important that along with
>>>>> this
>>>>> RFC you provide some comparisons with with other HW / SW APIs in order
>>>>> to help justify the design decisions. I also think there should be
>>>>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
>>>>
>>>> I agree with Nicolas.
>>>>
>>>> Regards,
>>>>
>>>>     Hans
>>>>
>>>>>
>>>>> regards,
>>>>> Nicolas
>>>>>
>>>>>>
>>>>>> Dikshita Agarwal (1):
>>>>>>   media: v4l2-ctrls:  add control for ltr
>>>>>>
>>>>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>>>>  2 files changed, 10 insertions(+)
>>>>>>
>>>>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-07-22 14:26           ` Hans Verkuil
@ 2020-08-06 11:05             ` dikshita
  2020-08-07  8:39               ` Hans Verkuil
  0 siblings, 1 reply; 11+ messages in thread
From: dikshita @ 2020-08-06 11:05 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

On 2020-07-22 19:56, Hans Verkuil wrote:
> On 20/07/2020 16:33, dikshita@codeaurora.org wrote:
>> On 2020-07-16 14:25, Hans Verkuil wrote:
>>> On 16/06/2020 20:41, dikshita@codeaurora.org wrote:
>>>> Hi Hans, Nicolas,
>>>> 
>>>> Thanks for your comments.
>>>> 
>>>> On 2020-06-12 14:41, Hans Verkuil wrote:
>>>>> Hi Dikshita, Nicolas,
>>>>> 
>>>>> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>>>>>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>>>>>> LTR (Long Term Reference) frames are the frames that are encoded
>>>>>>> sometime in the past
>>>>>>> and stored in the DPB buffer list to be used as reference to 
>>>>>>> encode
>>>>>>> future frames.
>>>>>>> One usage of LTR encoding is to reduce error propagation for 
>>>>>>> video
>>>>>>> transmission
>>>>>>> in packet lossy networks.  For example, encoder may want to 
>>>>>>> specify
>>>>>>> some key frames as
>>>>>>> LTR pictures and use them as reference frames for encoding. With
>>>>>>> extra protection
>>>>>>> selectively on these LTR frames or synchronization with the 
>>>>>>> receiver
>>>>>>> of reception of
>>>>>>> the LTR frames during transmission, decoder can receive reference
>>>>>>> frames more reliably
>>>>>>> than other non-reference frames. As a result, transmission error 
>>>>>>> can
>>>>>>> be effectively
>>>>>>> restricted within certain frames rather than propagated to future
>>>>>>> frames.
>>>>>>> 
>>>>>>> We are introducing below V4l2 Controls for this feature
>>>>>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>>>>>     a. This is used to query or configure the number of LTR 
>>>>>>> frames.
>>>>>>>        This is a static control and is controlled by the client.
>>>>>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>>>>>     c. If LTR Count is more than max supported LTR count (max 
>>>>>>> LTR) by
>>>>>>> driver, it will be rejected.
>>>>>>>     d. Auto Marking : If LTR count is non zero,
>>>>>>>         1) first LTR count frames would be mark as LTR 
>>>>>>> automatically
>>>>>>> after
>>>>>>>              every IDR frame (inclusive).
>>>>>>>         2) For multilayer encoding: first LTR count base layer
>>>>>>> reference frames starting after
>>>>>>>            every IDR frame (inclusive) in encoding order would be
>>>>>>> marked as LTR frames by the encoder.
>>>>>>>         3) Auto marking of LTR due to IDR should consider 
>>>>>>> following
>>>>>>> conditions:
>>>>>>>             1. The frame is not already set to be marked as LTR.
>>>>>>>             2. The frame is part of the base layer in the
>>>>>>> hierarchical layer case.
>>>>>>>             3. The number of frames currently marked as LTR is 
>>>>>>> less
>>>>>>> than the maximum LTR frame index plus 1.
>>>>>>>     e. Encoder needs to handle explicit Mark/Use command when 
>>>>>>> encoder
>>>>>>> is still doing "auto" marking
>>>>> 
>>>>> I don't follow this, quite possibly due to lack of experience with
>>>>> encoders.
>>>>> 
>>>>> I kind of would expect to see two modes: either automatic where
>>>>> encoders can
>>>>> mark up to LTR_COUNT frames as long term reference, and userspace 
>>>>> just
>>>>> sets
>>>>> LTR_COUNT and doesn't have to do anything else.
>>>>> 
>>>>> Or it is manual mode where userspace explicitly marks long term
>>>>> reference
>>>>> frames.
>>>>> 
>>>>> From the proposal above it looks like you can mix auto and manual
>>>>> modes.
>>>>> 
>>>>> BTW, how do you 'unmark' long term reference frames?
>>>>> 
>>>>> This feature is for stateful encoders, right?
>>>>> 
>>>>>> 
>>>>>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some 
>>>>>> encoder
>>>>>> can select by themself long term references and even some encoders 
>>>>>> may
>>>>>> not let the user decide.
>>>>>> 
>>>>>> (not huge han of LTR acronyme, but that could be fine too, 
>>>>>> assuming
>>>>>> you
>>>>>> add more _).
>>>>>> 
>>>> 
>>>> Userspace sets LTR count which signifies the number of LTR frames
>>>> encoder needs to generate or keep.
>>>> The encoder has to build-up its internal buffer reference list (aka 
>>>> DBP
>>>> list or recon buffer list).
>>>> In order to achieve that encoder will fill It's LTR (long term
>>>> references) list and STR (short term references) list
>>>> by auto marking n frames as LTR frames(n is equal to LTR count) 
>>>> based on
>>>> auto-marking dictated by the encoder spec.
>>>> The client then can replace those automatically marked frames with 
>>>> new
>>>> frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
>>>> encoder to refer the newly marked frame for encoding the next frame
>>>> using V4L2_CID_MPEG_VIDEO_USELTRFRAME.
>>>> 
>>>>>>> 
>>>>>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>>>>>     a. This signals to mark the current frame as LTR frame. It is 
>>>>>>> a
>>>>>>> dynamic control and also provide the LTR index to be used.
>>>>>>>     b. the LTR index provided by this control should never exceed 
>>>>>>> the
>>>>>>> max LTR-1. Else it will be rejected.
>>>>>> 
>>>>>> The "current" frame seems a bit loose. Perhaps you wanted to use
>>>>>> buffer
>>>>>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>>>>>> alternate interlacing.
>>>>> 
>>>>> I was thinking the same thing. Using a control for this doesn't 
>>>>> seem
>>>>> right.
>>>>> 
>>>> 
>>>> the client sets this to replace automatically marked frames by the
>>>> encoder with a particular frame.
>>>> this provides an index that ranges from 0 to LTR count-1 and then 
>>>> the
>>>> particular frame will be marked with that index.
>>>> this can be achieved through request by associating this control 
>>>> with a
>>>> specific buffer to make it synchronized.
>>>> 
>>>>>> 
>>>>>>> 
>>>>>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>>>>>     a. This specifies the LTR frame(s) to be used for encoding 
>>>>>>> the
>>>>>>> current frame. This is a dynamic control.
>>>>>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of 
>>>>>>> N
>>>>>>> LSB bits of this field are valid,
>>>>>>>        where N is the maximum number of LTRs supported. All the 
>>>>>>> other
>>>>>>> bits are invalid and should be rejected.
>>>>>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the 
>>>>>>> LSB
>>>>>>> corresponds to the LTR index max LTR-1.
>>>>> 
>>>>> How would userspace know this? Esp. with auto marking since 
>>>>> userspace
>>>>> would have
>>>>> to predict how auto marking works (if I understand this correctly).
>>>>> 
>>>> 
>>>> Client sets LTR count which tells about the number of LTR frames
>>>> automatically marked by the encoder.
>>>> so client can use LTR index (0 to LTR count -1) to ask encoder to 
>>>> refer
>>>> any particular
>>>> frame (marked automatically by driver or marked by client with
>>>> V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next
>>>> frame.
>>>> 
>>>>> For which HW encoder is this meant?
>>>>> 
>>>> This is primarily meant for H264 and HEVC.
>>> 
>>> The venus encoder?
>> yes
>>> 
>>> Some more questions:
>>> 
>>> 1) How many LTR frames do h.264 and hevc allow?
>> According to spec, MAX LTR allowed by H.264 is 15 and HEVC allows max 
>> 32 LTR frames.
>>> 2) Given N LTR frames, is there a ordering of those frames? E.g.
>>>    the LTR frame with index 0 is processed/used differently from
>>>    LTR frame with index 1? Or are they all equal in that it is just a 
>>> pool
>>>    of LTR frames that the encoder can use as it wishes?
>> they are different frames marked to be used as LTR and stored in 
>> available indices.
>> Userspace notifies encoder which LTR frame index to use via USE LTR 
>> control.
> 
> One more question: if a frame is marked as a LTR frame, does that mean
> that userspace
> can't reuse the buffer containing that frame? I assume that's the case.
> 
> One thing that I don't like about this API is that it introduces the
> LTR indices.
> Would it be possible to instead use buffer indices? (i.e. the
> v4l2_buffer index field).
> Then you can use a buffer flag to indicate that a buffer should be an
> LTR buffer,
> or (for automarking) the driver can mark a buffer as an LTR buffer by
> setting this flag
> before returning the buffer to userspace.
> 
> You would still need a V4L2_CID_MPEG_VIDEO_USELTRFRAME, but that can
> be a bitmask
> using the buffer index.
> 
> This would avoid introducing a second method of referring to buffers
> (LTR index),
> and I think that V4L2_CID_MPEG_VIDEO_MARKLTRFRAME can be dropped.
> 
> Regards,
> 
> 	Hans

Hi Hans,
LTR indices are part of the spec, we are not introducing anything new.
Also, there is no relation between v4l2 buffer index and LTR index. So 
we can't use V4l2 index as LTR index.

below are some details about LTR from spec:

Regarding LTR count:
max_num_ref_frames specifies the maximum number of short-term and 
long-term reference frames, complementary reference field pairs, and 
non-paired reference fields that may be used by the decoding process for 
inter prediction of any picture in the coded video sequence. The value 
of max_num_ref_frames shall be in the range of 0 to MaxDpbFrames (as 
specified in clause A.3.1 or A.3.2), inclusive.
max_long_term_frame_idx_plus1 minus 1 specifies the maximum value of 
long-term frame index allowed for long-term reference pictures (until 
receipt of another value of max_long_term_frame_idx_plus1). The value of 
max_long_term_frame_idx_plus1 shall be in the range of 0 to 
max_num_ref_frames, inclusive.


Regarding LTR index which will be specified by 
V4L2_CID_MPEG_VIDEO_MARKLTRFRAM :
long_term_pic_num specifies the long-term picture number of the picture 
being moved to the current index in the list. When decoding a coded 
frame, long_term_pic_num shall be equal to a LongTermPicNum assigned to 
one of the reference frames or complementary reference field pairs 
marked as "used for long-term reference".
long_term_frame_idx is used (with memory_management_control_operation 
equal to 3 or 6) to assign a long-term frame index to a picture. When 
the associated memory_management_control_operation is processed by the 
decoding process, the value of long_term_frame_idx shall be in the range 
of 0 to MaxLongTermFrameIdx, inclusive.


Also As there can be multiple LTR present. We can instruct encoder to 
use one and/or multiple of them using V4L2_CID_MPEG_VIDEO_USELTRFRAME 
with the bitmask.

Thanks,
Dikshita

> 
>> 
>> Thanks,
>> Dikshita
>>> 
>>> Regards,
>>> 
>>>     Hans
>>> 
>>>> 
>>>> Thanks,
>>>> Dikshita
>>>> 
>>>>>> 
>>>>>> Note, I haven't captured very well the userspace control flow, 
>>>>>> perhaps
>>>>>> this could be enhanced through writing some documentation.
>>>>>> 
>>>>>> As per all other generic encoder controls, we need to make sure it
>>>>>> will
>>>>>> be usable and flexible enough for multiple HW blocks, as it can be
>>>>>> tedious to extend later otherwise. It is important that along with
>>>>>> this
>>>>>> RFC you provide some comparisons with with other HW / SW APIs in 
>>>>>> order
>>>>>> to help justify the design decisions. I also think there should be
>>>>>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
>>>>> 
>>>>> I agree with Nicolas.
>>>>> 
>>>>> Regards,
>>>>> 
>>>>>     Hans
>>>>> 
>>>>>> 
>>>>>> regards,
>>>>>> Nicolas
>>>>>> 
>>>>>>> 
>>>>>>> Dikshita Agarwal (1):
>>>>>>>   media: v4l2-ctrls:  add control for ltr
>>>>>>> 
>>>>>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>>>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>>>>>  2 files changed, 10 insertions(+)
>>>>>>> 
>>>>>> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 0/1] Add LTR controls
  2020-08-06 11:05             ` dikshita
@ 2020-08-07  8:39               ` Hans Verkuil
  0 siblings, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2020-08-07  8:39 UTC (permalink / raw)
  To: dikshita
  Cc: Nicolas Dufresne, mchehab, ezequiel, boris.brezillon, ribalda,
	paul.kocialkowski, posciak, linux-media, stanimir.varbanov,
	linux-kernel, linux-arm-msm, vgarodia, majja, linux-media-owner

On 06/08/2020 13:05, dikshita@codeaurora.org wrote:
> On 2020-07-22 19:56, Hans Verkuil wrote:
>> On 20/07/2020 16:33, dikshita@codeaurora.org wrote:
>>> On 2020-07-16 14:25, Hans Verkuil wrote:
>>>> On 16/06/2020 20:41, dikshita@codeaurora.org wrote:
>>>>> Hi Hans, Nicolas,
>>>>>
>>>>> Thanks for your comments.
>>>>>
>>>>> On 2020-06-12 14:41, Hans Verkuil wrote:
>>>>>> Hi Dikshita, Nicolas,
>>>>>>
>>>>>> On 11/06/2020 16:22, Nicolas Dufresne wrote:
>>>>>>> Le jeudi 11 juin 2020 à 15:55 +0530, Dikshita Agarwal a écrit :
>>>>>>>> LTR (Long Term Reference) frames are the frames that are encoded
>>>>>>>> sometime in the past
>>>>>>>> and stored in the DPB buffer list to be used as reference to encode
>>>>>>>> future frames.
>>>>>>>> One usage of LTR encoding is to reduce error propagation for video
>>>>>>>> transmission
>>>>>>>> in packet lossy networks.  For example, encoder may want to specify
>>>>>>>> some key frames as
>>>>>>>> LTR pictures and use them as reference frames for encoding. With
>>>>>>>> extra protection
>>>>>>>> selectively on these LTR frames or synchronization with the receiver
>>>>>>>> of reception of
>>>>>>>> the LTR frames during transmission, decoder can receive reference
>>>>>>>> frames more reliably
>>>>>>>> than other non-reference frames. As a result, transmission error can
>>>>>>>> be effectively
>>>>>>>> restricted within certain frames rather than propagated to future
>>>>>>>> frames.
>>>>>>>>
>>>>>>>> We are introducing below V4l2 Controls for this feature
>>>>>>>> 1. V4L2_CID_MPEG_VIDEO_LTRCOUNT
>>>>>>>>     a. This is used to query or configure the number of LTR frames.
>>>>>>>>        This is a static control and is controlled by the client.
>>>>>>>>     b. The LTR index varies from 0 to the max LTR-1.
>>>>>>>>     c. If LTR Count is more than max supported LTR count (max LTR) by
>>>>>>>> driver, it will be rejected.
>>>>>>>>     d. Auto Marking : If LTR count is non zero,
>>>>>>>>         1) first LTR count frames would be mark as LTR automatically
>>>>>>>> after
>>>>>>>>              every IDR frame (inclusive).
>>>>>>>>         2) For multilayer encoding: first LTR count base layer
>>>>>>>> reference frames starting after
>>>>>>>>            every IDR frame (inclusive) in encoding order would be
>>>>>>>> marked as LTR frames by the encoder.
>>>>>>>>         3) Auto marking of LTR due to IDR should consider following
>>>>>>>> conditions:
>>>>>>>>             1. The frame is not already set to be marked as LTR.
>>>>>>>>             2. The frame is part of the base layer in the
>>>>>>>> hierarchical layer case.
>>>>>>>>             3. The number of frames currently marked as LTR is less
>>>>>>>> than the maximum LTR frame index plus 1.
>>>>>>>>     e. Encoder needs to handle explicit Mark/Use command when encoder
>>>>>>>> is still doing "auto" marking
>>>>>>
>>>>>> I don't follow this, quite possibly due to lack of experience with
>>>>>> encoders.
>>>>>>
>>>>>> I kind of would expect to see two modes: either automatic where
>>>>>> encoders can
>>>>>> mark up to LTR_COUNT frames as long term reference, and userspace just
>>>>>> sets
>>>>>> LTR_COUNT and doesn't have to do anything else.
>>>>>>
>>>>>> Or it is manual mode where userspace explicitly marks long term
>>>>>> reference
>>>>>> frames.
>>>>>>
>>>>>> From the proposal above it looks like you can mix auto and manual
>>>>>> modes.
>>>>>>
>>>>>> BTW, how do you 'unmark' long term reference frames?
>>>>>>
>>>>>> This feature is for stateful encoders, right?
>>>>>>
>>>>>>>
>>>>>>> Perhaps we are missing a LONG_TERM_REFERENCE_MODE ? I bet some encoder
>>>>>>> can select by themself long term references and even some encoders may
>>>>>>> not let the user decide.
>>>>>>>
>>>>>>> (not huge han of LTR acronyme, but that could be fine too, assuming
>>>>>>> you
>>>>>>> add more _).
>>>>>>>
>>>>>
>>>>> Userspace sets LTR count which signifies the number of LTR frames
>>>>> encoder needs to generate or keep.
>>>>> The encoder has to build-up its internal buffer reference list (aka DBP
>>>>> list or recon buffer list).
>>>>> In order to achieve that encoder will fill It's LTR (long term
>>>>> references) list and STR (short term references) list
>>>>> by auto marking n frames as LTR frames(n is equal to LTR count) based on
>>>>> auto-marking dictated by the encoder spec.
>>>>> The client then can replace those automatically marked frames with new
>>>>> frames using V4L2_CID_MPEG_VIDEO_MARKLTRFRAME and can ask
>>>>> encoder to refer the newly marked frame for encoding the next frame
>>>>> using V4L2_CID_MPEG_VIDEO_USELTRFRAME.
>>>>>
>>>>>>>>
>>>>>>>> 2. V4L2_CID_MPEG_VIDEO_MARKLTRFRAME :
>>>>>>>>     a. This signals to mark the current frame as LTR frame. It is a
>>>>>>>> dynamic control and also provide the LTR index to be used.
>>>>>>>>     b. the LTR index provided by this control should never exceed the
>>>>>>>> max LTR-1. Else it will be rejected.
>>>>>>>
>>>>>>> The "current" frame seems a bit loose. Perhaps you wanted to use
>>>>>>> buffer
>>>>>>> flags ? A bit like what we have to signal TOP/BOTTOM fields in
>>>>>>> alternate interlacing.
>>>>>>
>>>>>> I was thinking the same thing. Using a control for this doesn't seem
>>>>>> right.
>>>>>>
>>>>>
>>>>> the client sets this to replace automatically marked frames by the
>>>>> encoder with a particular frame.
>>>>> this provides an index that ranges from 0 to LTR count-1 and then the
>>>>> particular frame will be marked with that index.
>>>>> this can be achieved through request by associating this control with a
>>>>> specific buffer to make it synchronized.
>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> 3. V4L2_CID_MPEG_VIDEO_USELTRFRAME :
>>>>>>>>     a. This specifies the LTR frame(s) to be used for encoding the
>>>>>>>> current frame. This is a dynamic control.
>>>>>>>>     b. LTR Use Bitmap : this consists of bits [0, 15]. A total of N
>>>>>>>> LSB bits of this field are valid,
>>>>>>>>        where N is the maximum number of LTRs supported. All the other
>>>>>>>> bits are invalid and should be rejected.
>>>>>>>>        The LSB corresponds to the LTR index 0. Bit N-1 from the LSB
>>>>>>>> corresponds to the LTR index max LTR-1.
>>>>>>
>>>>>> How would userspace know this? Esp. with auto marking since userspace
>>>>>> would have
>>>>>> to predict how auto marking works (if I understand this correctly).
>>>>>>
>>>>>
>>>>> Client sets LTR count which tells about the number of LTR frames
>>>>> automatically marked by the encoder.
>>>>> so client can use LTR index (0 to LTR count -1) to ask encoder to refer
>>>>> any particular
>>>>> frame (marked automatically by driver or marked by client with
>>>>> V4L2_CID_MPEG_VIDEO_MARKLTRFRAME) as a reference to encode the next
>>>>> frame.
>>>>>
>>>>>> For which HW encoder is this meant?
>>>>>>
>>>>> This is primarily meant for H264 and HEVC.
>>>>
>>>> The venus encoder?
>>> yes
>>>>
>>>> Some more questions:
>>>>
>>>> 1) How many LTR frames do h.264 and hevc allow?
>>> According to spec, MAX LTR allowed by H.264 is 15 and HEVC allows max 32 LTR frames.
>>>> 2) Given N LTR frames, is there a ordering of those frames? E.g.
>>>>    the LTR frame with index 0 is processed/used differently from
>>>>    LTR frame with index 1? Or are they all equal in that it is just a pool
>>>>    of LTR frames that the encoder can use as it wishes?
>>> they are different frames marked to be used as LTR and stored in available indices.
>>> Userspace notifies encoder which LTR frame index to use via USE LTR control.
>>
>> One more question: if a frame is marked as a LTR frame, does that mean
>> that userspace
>> can't reuse the buffer containing that frame? I assume that's the case.
>>
>> One thing that I don't like about this API is that it introduces the
>> LTR indices.
>> Would it be possible to instead use buffer indices? (i.e. the
>> v4l2_buffer index field).
>> Then you can use a buffer flag to indicate that a buffer should be an
>> LTR buffer,
>> or (for automarking) the driver can mark a buffer as an LTR buffer by
>> setting this flag
>> before returning the buffer to userspace.
>>
>> You would still need a V4L2_CID_MPEG_VIDEO_USELTRFRAME, but that can
>> be a bitmask
>> using the buffer index.
>>
>> This would avoid introducing a second method of referring to buffers
>> (LTR index),
>> and I think that V4L2_CID_MPEG_VIDEO_MARKLTRFRAME can be dropped.
>>
>> Regards,
>>
>>     Hans
> 
> Hi Hans,
> LTR indices are part of the spec, we are not introducing anything new.
> Also, there is no relation between v4l2 buffer index and LTR index. So we can't use V4l2 index as LTR index.

Ah, I think I start to understand the cause of my confusion: I thought that the
buffers containing LTR frames had to be kept around by userspace, but instead the
driver/HW makes copies and userspace does not need to do anything special with
those buffers.

OK, can you post a v2?

Document the controls in Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

Regards,

	Hans

> 
> below are some details about LTR from spec:
> 
> Regarding LTR count:
> max_num_ref_frames specifies the maximum number of short-term and long-term reference frames, complementary reference field pairs, and non-paired reference fields that may be used by the decoding
> process for inter prediction of any picture in the coded video sequence. The value of max_num_ref_frames shall be in the range of 0 to MaxDpbFrames (as specified in clause A.3.1 or A.3.2), inclusive.
> max_long_term_frame_idx_plus1 minus 1 specifies the maximum value of long-term frame index allowed for long-term reference pictures (until receipt of another value of max_long_term_frame_idx_plus1).
> The value of max_long_term_frame_idx_plus1 shall be in the range of 0 to max_num_ref_frames, inclusive.
> 
> 
> Regarding LTR index which will be specified by V4L2_CID_MPEG_VIDEO_MARKLTRFRAM :
> long_term_pic_num specifies the long-term picture number of the picture being moved to the current index in the list. When decoding a coded frame, long_term_pic_num shall be equal to a LongTermPicNum
> assigned to one of the reference frames or complementary reference field pairs marked as "used for long-term reference".
> long_term_frame_idx is used (with memory_management_control_operation equal to 3 or 6) to assign a long-term frame index to a picture. When the associated memory_management_control_operation is
> processed by the decoding process, the value of long_term_frame_idx shall be in the range of 0 to MaxLongTermFrameIdx, inclusive.
> 
> 
> Also As there can be multiple LTR present. We can instruct encoder to use one and/or multiple of them using V4L2_CID_MPEG_VIDEO_USELTRFRAME with the bitmask.
> 
> Thanks,
> Dikshita
> 
>>
>>>
>>> Thanks,
>>> Dikshita
>>>>
>>>> Regards,
>>>>
>>>>     Hans
>>>>
>>>>>
>>>>> Thanks,
>>>>> Dikshita
>>>>>
>>>>>>>
>>>>>>> Note, I haven't captured very well the userspace control flow, perhaps
>>>>>>> this could be enhanced through writing some documentation.
>>>>>>>
>>>>>>> As per all other generic encoder controls, we need to make sure it
>>>>>>> will
>>>>>>> be usable and flexible enough for multiple HW blocks, as it can be
>>>>>>> tedious to extend later otherwise. It is important that along with
>>>>>>> this
>>>>>>> RFC you provide some comparisons with with other HW / SW APIs in order
>>>>>>> to help justify the design decisions. I also think there should be
>>>>>>> link made V4L2_CID_MPEG_VIDEO_GOP_* , number of B-Frames etc.
>>>>>>
>>>>>> I agree with Nicolas.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>>     Hans
>>>>>>
>>>>>>>
>>>>>>> regards,
>>>>>>> Nicolas
>>>>>>>
>>>>>>>>
>>>>>>>> Dikshita Agarwal (1):
>>>>>>>>   media: v4l2-ctrls:  add control for ltr
>>>>>>>>
>>>>>>>>  drivers/media/v4l2-core/v4l2-ctrls.c | 6 ++++++
>>>>>>>>  include/uapi/linux/v4l2-controls.h   | 4 ++++
>>>>>>>>  2 files changed, 10 insertions(+)
>>>>>>>>
>>>>>>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-08-07  8:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 10:25 [RFC PATCH 0/1] Add LTR controls Dikshita Agarwal
2020-06-11 10:25 ` [RFC PATCH 1/1] media: v4l2-ctrls: add control for ltr Dikshita Agarwal
2020-06-11 14:22 ` [RFC PATCH 0/1] Add LTR controls Nicolas Dufresne
2020-06-12  9:11   ` Hans Verkuil
2020-06-16 18:41     ` dikshita
2020-07-14  7:21       ` dikshita
2020-07-16  8:55       ` Hans Verkuil
2020-07-20 14:33         ` dikshita
2020-07-22 14:26           ` Hans Verkuil
2020-08-06 11:05             ` dikshita
2020-08-07  8:39               ` 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).