MEM2MEM devices: how to handle sequence number?

Message ID CACKLOr0H4enuADtWcUkZCS_V92mmLD8K5CgScbGo7w9nbT=-CA@mail.gmail.com (mailing list archive)
State RFC, archived
Headers

Commit Message

Javier Martin Dec. 22, 2011, 2:34 p.m. UTC
  Hi,
we have a processing chain composed of three v4l2 devices:

---------------------           -----------------------
----------------------
| v4l2 source  |            |     v4l2 fixer   |               |  v4l2 encoder |
|  (capture)     |---------->|  (mem2mem)| ------------>|  (mem2mem) |
------------>
|___________|            |____________|              |____________|


"v4l2 source" generates consecutive sequence numbers so that we can
detect whether a frame has been lost or not.
"v4l2 fixer" and "v4l2 encoder" cannot lose frames because they don't
interact with an external sensor.

How should "v4l2 fixer" and "v4l2 encoder" behave regarding frame
sequence number? Should they just copy the sequence number from the
input buffer to the output buffer or should they maintain their own
count for the CAPTURE queue?

If the former option is chosen we should apply a patch like the
following so that the sequence number of the input buffer is passed to
the videobuf2 layer:


Regards.
  

Comments

Marek Szyprowski Dec. 23, 2011, 7:09 a.m. UTC | #1
Hello,

On Thursday, December 22, 2011 3:34 PM Javier Martin wrote:

> we have a processing chain composed of three v4l2 devices:
> 
> ---------------------           -----------------------
> ----------------------
> | v4l2 source  |            |     v4l2 fixer   |               |  v4l2 encoder |
> |  (capture)     |---------->|  (mem2mem)| ------------>|  (mem2mem) |
> ------------>
> |___________|            |____________|              |____________|
> 
> 
> "v4l2 source" generates consecutive sequence numbers so that we can
> detect whether a frame has been lost or not.
> "v4l2 fixer" and "v4l2 encoder" cannot lose frames because they don't
> interact with an external sensor.
> 
> How should "v4l2 fixer" and "v4l2 encoder" behave regarding frame
> sequence number? Should they just copy the sequence number from the
> input buffer to the output buffer or should they maintain their own
> count for the CAPTURE queue?

IMHO mem2mem devices, which process buffers in 1:1 way (there is always 
exactly one 'capture'/destination buffer for every 'output'/source buffer)
can simply copy the sequence number from the source buffer to the
destination.

If there is no such 1:1 mapping between the buffers, drivers should maintain
their own numbers. video encoder is probably an example of such device.
A single destination ('capture') buffer with encoded video data might 
contain a fraction, one or more source ('output') video buffers/frames.

> If the former option is chosen we should apply a patch like the
> following so that the sequence number of the input buffer is passed to
> the videobuf2 layer:
> 
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 1250662..7d8a88b 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>          */
>         list_add_tail(&vb->queued_entry, &q->queued_list);
>         vb->state = VB2_BUF_STATE_QUEUED;
> +       vb->v4l2_buf.sequence = b->sequence;
> 
>         /*
>          * If already streaming, give the buffer to driver for processing.
> 

Right, such patch is definitely needed. Please resend it with 'signed-off-by'
annotation.

Best regards
  
Laurent Pinchart Dec. 23, 2011, 11:28 a.m. UTC | #2
Hi Marek,

On Friday 23 December 2011 08:09:25 Marek Szyprowski wrote:
> On Thursday, December 22, 2011 3:34 PM Javier Martin wrote:
> > we have a processing chain composed of three v4l2 devices:
> > 
> > ---------------------           -----------------------
> > ----------------------
> > 
> > | v4l2 source  |            |     v4l2 fixer   |               |  v4l2
> > | encoder |
> > | 
> > |  (capture)     |---------->|  (mem2mem)| ------------>|  (mem2mem) |
> > 
> > ------------>
> > 
> > |___________|            |____________|              |____________|
> > 
> > "v4l2 source" generates consecutive sequence numbers so that we can
> > detect whether a frame has been lost or not.
> > "v4l2 fixer" and "v4l2 encoder" cannot lose frames because they don't
> > interact with an external sensor.
> > 
> > How should "v4l2 fixer" and "v4l2 encoder" behave regarding frame
> > sequence number? Should they just copy the sequence number from the
> > input buffer to the output buffer or should they maintain their own
> > count for the CAPTURE queue?
> 
> IMHO mem2mem devices, which process buffers in 1:1 way (there is always
> exactly one 'capture'/destination buffer for every 'output'/source buffer)
> can simply copy the sequence number from the source buffer to the
> destination.
> 
> If there is no such 1:1 mapping between the buffers, drivers should
> maintain their own numbers. video encoder is probably an example of such
> device. A single destination ('capture') buffer with encoded video data
> might contain a fraction, one or more source ('output') video
> buffers/frames.
> 
> > If the former option is chosen we should apply a patch like the
> > following so that the sequence number of the input buffer is passed to
> > the videobuf2 layer:
> > 
> > diff --git a/drivers/media/video/videobuf2-core.c
> > b/drivers/media/video/videobuf2-core.c
> > index 1250662..7d8a88b 100644
> > --- a/drivers/media/video/videobuf2-core.c
> > +++ b/drivers/media/video/videobuf2-core.c
> > @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct
> > v4l2_buffer *b)
> >          */
> >         list_add_tail(&vb->queued_entry, &q->queued_list);
> >         vb->state = VB2_BUF_STATE_QUEUED;
> > +       vb->v4l2_buf.sequence = b->sequence;
> >         /*
> >          * If already streaming, give the buffer to driver for
> >          processing.
> 
> Right, such patch is definitely needed. Please resend it with
> 'signed-off-by' annotation.

I'm not too sure about that. Isn't the sequence number supposed to be ignored 
by drivers on video output devices ? The documentation is a bit terse on the 
subject, all it says is

__u32  sequence     Set by the driver, counting the frames in the sequence.
  
Marek Szyprowski Dec. 23, 2011, 11:35 a.m. UTC | #3
Hello,

On Friday, December 23, 2011 12:29 PM Laurent Pinchart wrote:
> On Friday 23 December 2011 08:09:25 Marek Szyprowski wrote:
> > On Thursday, December 22, 2011 3:34 PM Javier Martin wrote:
> > > we have a processing chain composed of three v4l2 devices:
> > >
> > > ---------------------           -----------------------
> > > ----------------------
> > >
> > > | v4l2 source  |            |     v4l2 fixer   |               |  v4l2
> > > | encoder |
> > > |
> > > |  (capture)     |---------->|  (mem2mem)| ------------>|  (mem2mem) |
> > >
> > > ------------>
> > >
> > > |___________|            |____________|              |____________|
> > >
> > > "v4l2 source" generates consecutive sequence numbers so that we can
> > > detect whether a frame has been lost or not.
> > > "v4l2 fixer" and "v4l2 encoder" cannot lose frames because they don't
> > > interact with an external sensor.
> > >
> > > How should "v4l2 fixer" and "v4l2 encoder" behave regarding frame
> > > sequence number? Should they just copy the sequence number from the
> > > input buffer to the output buffer or should they maintain their own
> > > count for the CAPTURE queue?
> >
> > IMHO mem2mem devices, which process buffers in 1:1 way (there is always
> > exactly one 'capture'/destination buffer for every 'output'/source buffer)
> > can simply copy the sequence number from the source buffer to the
> > destination.
> >
> > If there is no such 1:1 mapping between the buffers, drivers should
> > maintain their own numbers. video encoder is probably an example of such
> > device. A single destination ('capture') buffer with encoded video data
> > might contain a fraction, one or more source ('output') video
> > buffers/frames.
> >
> > > If the former option is chosen we should apply a patch like the
> > > following so that the sequence number of the input buffer is passed to
> > > the videobuf2 layer:
> > >
> > > diff --git a/drivers/media/video/videobuf2-core.c
> > > b/drivers/media/video/videobuf2-core.c
> > > index 1250662..7d8a88b 100644
> > > --- a/drivers/media/video/videobuf2-core.c
> > > +++ b/drivers/media/video/videobuf2-core.c
> > > @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct
> > > v4l2_buffer *b)
> > >          */
> > >         list_add_tail(&vb->queued_entry, &q->queued_list);
> > >         vb->state = VB2_BUF_STATE_QUEUED;
> > > +       vb->v4l2_buf.sequence = b->sequence;
> > >         /*
> > >          * If already streaming, give the buffer to driver for
> > >          processing.
> >
> > Right, such patch is definitely needed. Please resend it with
> > 'signed-off-by' annotation.
> 
> I'm not too sure about that. Isn't the sequence number supposed to be ignored
> by drivers on video output devices ? The documentation is a bit terse on the
> subject, all it says is
> 
> __u32  sequence     Set by the driver, counting the frames in the sequence.

We can also update the documentation if needed. IMHO copying sequence number
in mem2mem case if there is 1:1 relation between the buffers is a good idea.

Best regards
  
Laurent Pinchart Dec. 23, 2011, 11:54 a.m. UTC | #4
Hi Marek,

On Friday 23 December 2011 12:35:09 Marek Szyprowski wrote:
> On Friday, December 23, 2011 12:29 PM Laurent Pinchart wrote:
> > On Friday 23 December 2011 08:09:25 Marek Szyprowski wrote:
> > > On Thursday, December 22, 2011 3:34 PM Javier Martin wrote:
> > > > we have a processing chain composed of three v4l2 devices:
> > > > 
> > > > ---------------------           -----------------------
> > > > ----------------------
> > > > 
> > > > | v4l2 source  |            |     v4l2 fixer   |               | 
> > > > | v4l2 encoder |
> > > > | 
> > > > |  (capture)     |---------->|  (mem2mem)| ------------>|  (mem2mem)
> > > > |  |
> > > > 
> > > > ------------>
> > > > 
> > > > |___________|            |____________|              |____________|
> > > > 
> > > > "v4l2 source" generates consecutive sequence numbers so that we can
> > > > detect whether a frame has been lost or not.
> > > > "v4l2 fixer" and "v4l2 encoder" cannot lose frames because they don't
> > > > interact with an external sensor.
> > > > 
> > > > How should "v4l2 fixer" and "v4l2 encoder" behave regarding frame
> > > > sequence number? Should they just copy the sequence number from the
> > > > input buffer to the output buffer or should they maintain their own
> > > > count for the CAPTURE queue?
> > > 
> > > IMHO mem2mem devices, which process buffers in 1:1 way (there is always
> > > exactly one 'capture'/destination buffer for every 'output'/source
> > > buffer) can simply copy the sequence number from the source buffer to
> > > the destination.
> > > 
> > > If there is no such 1:1 mapping between the buffers, drivers should
> > > maintain their own numbers. video encoder is probably an example of
> > > such device. A single destination ('capture') buffer with encoded
> > > video data might contain a fraction, one or more source ('output')
> > > video
> > > buffers/frames.
> > > 
> > > > If the former option is chosen we should apply a patch like the
> > > > following so that the sequence number of the input buffer is passed
> > > > to the videobuf2 layer:
> > > > 
> > > > diff --git a/drivers/media/video/videobuf2-core.c
> > > > b/drivers/media/video/videobuf2-core.c
> > > > index 1250662..7d8a88b 100644
> > > > --- a/drivers/media/video/videobuf2-core.c
> > > > +++ b/drivers/media/video/videobuf2-core.c
> > > > @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct
> > > > v4l2_buffer *b)
> > > > 
> > > >          */
> > > >         
> > > >         list_add_tail(&vb->queued_entry, &q->queued_list);
> > > >         vb->state = VB2_BUF_STATE_QUEUED;
> > > > 
> > > > +       vb->v4l2_buf.sequence = b->sequence;
> > > > 
> > > >         /*
> > > >         
> > > >          * If already streaming, give the buffer to driver for
> > > >          processing.
> > > 
> > > Right, such patch is definitely needed. Please resend it with
> > > 'signed-off-by' annotation.
> > 
> > I'm not too sure about that. Isn't the sequence number supposed to be
> > ignored by drivers on video output devices ? The documentation is a bit
> > terse on the subject, all it says is
> > 
> > __u32  sequence     Set by the driver, counting the frames in the
> > sequence.
> 
> We can also update the documentation if needed. IMHO copying sequence
> number in mem2mem case if there is 1:1 relation between the buffers is a
> good idea.

My point is that sequence numbers are currently not applicable to video output 
devices, at least according to the documentation. Applications will just set 
them to 0.

I think it would be better to have the m2m driver set the sequence number 
internally on the video output node by incrementing an counter, and pass it 
down the pipeline to the video capture node.
  
Andy Walls Dec. 23, 2011, 11:57 a.m. UTC | #5
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

>Hello,
>
>On Friday, December 23, 2011 12:29 PM Laurent Pinchart wrote:
>> On Friday 23 December 2011 08:09:25 Marek Szyprowski wrote:
>> > On Thursday, December 22, 2011 3:34 PM Javier Martin wrote:
>> > > we have a processing chain composed of three v4l2 devices:
>> > >
>> > > ---------------------           -----------------------
>> > > ----------------------
>> > >
>> > > | v4l2 source  |            |     v4l2 fixer   |               | 
>v4l2
>> > > | encoder |
>> > > |
>> > > |  (capture)     |---------->|  (mem2mem)| ------------>| 
>(mem2mem) |
>> > >
>> > > ------------>
>> > >
>> > > |___________|            |____________|             
>|____________|
>> > >
>> > > "v4l2 source" generates consecutive sequence numbers so that we
>can
>> > > detect whether a frame has been lost or not.
>> > > "v4l2 fixer" and "v4l2 encoder" cannot lose frames because they
>don't
>> > > interact with an external sensor.
>> > >
>> > > How should "v4l2 fixer" and "v4l2 encoder" behave regarding frame
>> > > sequence number? Should they just copy the sequence number from
>the
>> > > input buffer to the output buffer or should they maintain their
>own
>> > > count for the CAPTURE queue?
>> >
>> > IMHO mem2mem devices, which process buffers in 1:1 way (there is
>always
>> > exactly one 'capture'/destination buffer for every 'output'/source
>buffer)
>> > can simply copy the sequence number from the source buffer to the
>> > destination.
>> >
>> > If there is no such 1:1 mapping between the buffers, drivers should
>> > maintain their own numbers. video encoder is probably an example of
>such
>> > device. A single destination ('capture') buffer with encoded video
>data
>> > might contain a fraction, one or more source ('output') video
>> > buffers/frames.
>> >
>> > > If the former option is chosen we should apply a patch like the
>> > > following so that the sequence number of the input buffer is
>passed to
>> > > the videobuf2 layer:
>> > >
>> > > diff --git a/drivers/media/video/videobuf2-core.c
>> > > b/drivers/media/video/videobuf2-core.c
>> > > index 1250662..7d8a88b 100644
>> > > --- a/drivers/media/video/videobuf2-core.c
>> > > +++ b/drivers/media/video/videobuf2-core.c
>> > > @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct
>> > > v4l2_buffer *b)
>> > >          */
>> > >         list_add_tail(&vb->queued_entry, &q->queued_list);
>> > >         vb->state = VB2_BUF_STATE_QUEUED;
>> > > +       vb->v4l2_buf.sequence = b->sequence;
>> > >         /*
>> > >          * If already streaming, give the buffer to driver for
>> > >          processing.
>> >
>> > Right, such patch is definitely needed. Please resend it with
>> > 'signed-off-by' annotation.
>> 
>> I'm not too sure about that. Isn't the sequence number supposed to be
>ignored
>> by drivers on video output devices ? The documentation is a bit terse
>on the
>> subject, all it says is
>> 
>> __u32  sequence     Set by the driver, counting the frames in the
>sequence.
>
>We can also update the documentation if needed. IMHO copying sequence
>number
>in mem2mem case if there is 1:1 relation between the buffers is a good
>idea.
>
>Best regards
>-- 
>Marek Szyprowski
>Samsung Poland R&D Center
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media"
>in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Could there ever be a case where the v4l2 source changes and causes a jump in the frame count at the encoder, which would then matter to an application?

Regards,
Andy 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Sylwester Nawrocki Dec. 23, 2011, 3:55 p.m. UTC | #6
Hi Laurent,

On 12/23/2011 12:54 PM, Laurent Pinchart wrote:
>>>>> diff --git a/drivers/media/video/videobuf2-core.c
>>>>> b/drivers/media/video/videobuf2-core.c
>>>>> index 1250662..7d8a88b 100644
>>>>> --- a/drivers/media/video/videobuf2-core.c
>>>>> +++ b/drivers/media/video/videobuf2-core.c
>>>>> @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct
>>>>> v4l2_buffer *b)
>>>>>
>>>>>           */
>>>>>
>>>>>          list_add_tail(&vb->queued_entry,&q->queued_list);
>>>>>          vb->state = VB2_BUF_STATE_QUEUED;
>>>>>
>>>>> +       vb->v4l2_buf.sequence = b->sequence;
>>>>>
>>>>>          /*
>>>>>
>>>>>           * If already streaming, give the buffer to driver for
>>>>>           processing.
>>>>
>>>> Right, such patch is definitely needed. Please resend it with
>>>> 'signed-off-by' annotation.
>>>
>>> I'm not too sure about that. Isn't the sequence number supposed to be
>>> ignored by drivers on video output devices ? The documentation is a bit
>>> terse on the subject, all it says is
>>>
>>> __u32  sequence     Set by the driver, counting the frames in the
>>> sequence.
>>
>> We can also update the documentation if needed. IMHO copying sequence
>> number in mem2mem case if there is 1:1 relation between the buffers is a
>> good idea.
> 
> My point is that sequence numbers are currently not applicable to video output
> devices, at least according to the documentation. Applications will just set
> them to 0.

Looks like the documentation wasn't updated when the Memory-To-Memory interface
has been introduced.
 
> I think it would be better to have the m2m driver set the sequence number
> internally on the video output node by incrementing an counter, and pass it
> down the pipeline to the video capture node.

It sounds reasonable. Currently the sequence is zeroed at streamon in the
capture drivers. Similar behaviour could be assured by m2m drivers.
In Javier's case it's probably more reliable to check the sequence numbers
contiguity directly at the image source driver's device node.   

Although when m2m driver sets the sequence number internally on a video
output queue it could make sense to have the buffer's sequence number updated
upon return from VIDIOC_QBUF. What do you think ?

This would be needed for the object detection interface if we wanted to 
associate object detection result with a frame sequence number.

As far as the implementation is concerned, m2m and output drivers (with selected
capabilities only?) would have to update buffer sequence number from within
buf_queue vb2 queue op.


--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Laurent Pinchart Dec. 25, 2011, 9:19 p.m. UTC | #7
Hi Sylwester,

On Friday 23 December 2011 16:55:10 Sylwester Nawrocki wrote:
> On 12/23/2011 12:54 PM, Laurent Pinchart wrote:
> >>>>> diff --git a/drivers/media/video/videobuf2-core.c
> >>>>> b/drivers/media/video/videobuf2-core.c
> >>>>> index 1250662..7d8a88b 100644
> >>>>> --- a/drivers/media/video/videobuf2-core.c
> >>>>> +++ b/drivers/media/video/videobuf2-core.c
> >>>>> @@ -1127,6 +1127,7 @@ int vb2_qbuf(struct vb2_queue *q, struct
> >>>>> v4l2_buffer *b)
> >>>>> 
> >>>>>           */
> >>>>>          
> >>>>>          list_add_tail(&vb->queued_entry,&q->queued_list);
> >>>>>          vb->state = VB2_BUF_STATE_QUEUED;
> >>>>> 
> >>>>> +       vb->v4l2_buf.sequence = b->sequence;
> >>>>> 
> >>>>>          /*
> >>>>>          
> >>>>>           * If already streaming, give the buffer to driver for
> >>>>>           processing.
> >>>> 
> >>>> Right, such patch is definitely needed. Please resend it with
> >>>> 'signed-off-by' annotation.
> >>> 
> >>> I'm not too sure about that. Isn't the sequence number supposed to be
> >>> ignored by drivers on video output devices ? The documentation is a bit
> >>> terse on the subject, all it says is
> >>> 
> >>> __u32  sequence     Set by the driver, counting the frames in the
> >>> sequence.
> >> 
> >> We can also update the documentation if needed. IMHO copying sequence
> >> number in mem2mem case if there is 1:1 relation between the buffers is a
> >> good idea.
> > 
> > My point is that sequence numbers are currently not applicable to video
> > output devices, at least according to the documentation. Applications
> > will just set them to 0.
> 
> Looks like the documentation wasn't updated when the Memory-To-Memory
> interface has been introduced.
> 
> > I think it would be better to have the m2m driver set the sequence number
> > internally on the video output node by incrementing an counter, and pass
> > it down the pipeline to the video capture node.
> 
> It sounds reasonable. Currently the sequence is zeroed at streamon in the
> capture drivers. Similar behaviour could be assured by m2m drivers.

I agree.

> In Javier's case it's probably more reliable to check the sequence numbers
> contiguity directly at the image source driver's device node.
> 
> Although when m2m driver sets the sequence number internally on a video
> output queue it could make sense to have the buffer's sequence number
> updated upon return from VIDIOC_QBUF. What do you think ?

Yes, that wouldn't hurt and could provide interesting information. In the 
general video output case it won't matter much, as the sequence numbers will 
be contiguous and won't be used by anything else, but in the mem-to-mem case 
it could let applications synchronize images with the video capture node on 
the mem-to-mem output. However, we can't always assume a 1-to-1 correspondance 
between an input (to the mem-to-mem device) and an output buffer, as more than 
one input buffer could be required to decompress a frame for instance. In that 
case, should the video capture node still report the sequence number used by 
the video output node ? If it does, there will be gaps in the video capture 
sequence numbers.

> This would be needed for the object detection interface if we wanted to
> associate object detection result with a frame sequence number.

Agreed, in that case that would be pretty interesting.

> As far as the implementation is concerned, m2m and output drivers (with
> selected capabilities only?) would have to update buffer sequence number
> from within buf_queue vb2 queue op.
  
Javier Martin Jan. 2, 2012, 10:22 a.m. UTC | #8
Hi,
i've just arrived the office after holidays and it seems you have
agreed some solution to the sequence number issue.

As I understand, for a case where there is 1:1 correspondence between
input and output (which is my case) I should do the following:

- keep an internal frame counter associated with the output queue.
- return the frame number when the user calls VIDIOC_QBUF on the output.
- pass the output frame number to the capture queue in a 1:1 basis

So in my chain of three processed nodes each node has its own internal
frame counter and frame loss should be checked at the video source.

Is that OK?
  
Laurent Pinchart Jan. 2, 2012, 11:06 a.m. UTC | #9
Hi Javier,

On Monday 02 January 2012 11:22:54 javier Martin wrote:
> Hi,
> i've just arrived the office after holidays and it seems you have
> agreed some solution to the sequence number issue.
> 
> As I understand, for a case where there is 1:1 correspondence between
> input and output (which is my case) I should do the following:
> 
> - keep an internal frame counter associated with the output queue.
> - return the frame number when the user calls VIDIOC_QBUF on the output.
> - pass the output frame number to the capture queue in a 1:1 basis

That's right.

> So in my chain of three processed nodes each node has its own internal
> frame counter and frame loss should be checked at the video source.

You can use an internal frame counter for each node if needed for internal 
operation, but that's not required from the userspace point of view.

> Is that OK?
  
Javier Martin Jan. 2, 2012, 11:44 a.m. UTC | #10
Just one more question about this.

The v4l2 encoder, which is the last element in my processing chain, is
an H.264 encoder that has to know about previous frames to encode.
For these kind of devices it is very useful to know whether a frame
has been lost to introduce a skip frame and improve the encoding
process.

But, with the current approach we don't have any way to communicate
this to the device.

One option would be that the user specified a sequence number when
issuing VIDIOC_QBUF at the output queue so that the device could
detect any discontinuity and introduce a skip frame. But this would
break your rule that sequence number introduced at the output queue
has to be ignored by the driver.
  
Laurent Pinchart Jan. 3, 2012, 2 p.m. UTC | #11
Hi Javier,

On Monday 02 January 2012 12:44:08 javier Martin wrote:
> Just one more question about this.
> 
> The v4l2 encoder, which is the last element in my processing chain, is
> an H.264 encoder that has to know about previous frames to encode.
> For these kind of devices it is very useful to know whether a frame
> has been lost to introduce a skip frame and improve the encoding
> process.
> 
> But, with the current approach we don't have any way to communicate
> this to the device.
> 
> One option would be that the user specified a sequence number when
> issuing VIDIOC_QBUF at the output queue so that the device could
> detect any discontinuity and introduce a skip frame. But this would
> break your rule that sequence number introduced at the output queue
> has to be ignored by the driver.

That's a use case I haven't thought about. Using sequence numbers could indeed 
help in that case. My concern is that most (if not all) applications don't set 
the sequence number before queuing a buffer, so requiring them to do so could 
introduce breakages. This could be limited to H.264 encoders (or more 
generally codecs) though.

Another option would be to queue a 0-bytes frame. That might be a bit hackish 
though.
  

Patch

diff --git a/drivers/media/video/videobuf2-core.c
b/drivers/media/video/videobuf2-core.c
index 1250662..7d8a88b 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -1127,6 +1127,7 @@  int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
         */
        list_add_tail(&vb->queued_entry, &q->queued_list);
        vb->state = VB2_BUF_STATE_QUEUED;
+       vb->v4l2_buf.sequence = b->sequence;

        /*
         * If already streaming, give the buffer to driver for processing.