On Wed, 2012-05-23 at 14:28 +0200, Hans Verkuil wrote: > On Wed 23 May 2012 13:20:03 Andrzej Hajda wrote: > > On Wed, 2012-05-23 at 09:43 +0200, Hans Verkuil wrote: > > > Hi Andrzej! > > > > > > Thanks for the patch, but I do have two questions: > > > > > > On Tue 22 May 2012 17:33:53 Andrzej Hajda wrote: > > > > Those patches add end of stream handling for s5p-mfc encoder. > > > > > > > > The first patch was sent already to the list as RFC, but the discussion ended > > > > without any decision. > > > > This patch adds new v4l2_buffer flag V4L2_BUF_FLAG_EOS. Below short > > > > description of this change. > > > > > > > > s5p_mfc is a mem-to-mem MPEG/H263/H264 encoder and it requires that the last > > > > incoming frame must be processed differently, it means the information about > > > > the end of the stream driver should receive NOT LATER than the last > > > > V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer. Common practice > > > > of sending empty buffer to indicate end-of-stream do not work in such case. > > > > Setting V4L2_BUF_FLAG_EOS flag for the last V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE > > > > buffer seems to be the most straightforward solution here. > > > > > > > > V4L2_BUF_FLAG_EOS flag should be used by application if driver requires it > > > > > > How will the application know that? > > > > Application can always set this flag, it will be ignored by drivers not > > requiring it. > > That's going to make it very hard to write generic applications: people will > always forget to set that flag, unless they happen to using your hardware. > > > I see some drawback of this solution - application should know if the > > frame enqueued to the driver is the last one. If the application > > receives frames to encode form an external source (for example via pipe) > > it often does not know if the frame it received is the last one. So to > > be able to properly queue frame to the driver it should wait with frame > > queuing until it knows there is next frame or end-of-stream is reached, > > in such situation it will properly set flag before queuing. > > > > Alternative to "V4L2_BUF_FLAG_EOS" solution is to implement "wait for > > next frame" logic directly into the driver. In such case application can > > use empty buffer to signal the end of the stream. Driver waits with > > frame processing if there are at least two buffers in output queue. Then > > it checks if the second buffer is empty if not it process the first > > buffer as a normal frame and repeats procedure, if yes it process the > > first buffer as the last frame and releases second buffer. > > In the current V4L2 API the last output frame is reached when: > > 1) the filehandle is closed > 2) VIDIOC_STREAMOFF is called > 3) VIDIOC_ENCODER_CMD is called with V4L2_ENC_CMD_STOP. > > The latter is currently only used by MPEG encoders, but it might be an idea > to consider it for your hardware as well. Perhaps a flag like 'stop_after_next_frame' > is needed. It seemed to me less straightforward - EOS is sent before the last frame - but I can implement it this way of course. > > How are cases 1 and 2 handled today? > As I lurked into the driver's code it seems it behaves in standard way - driver waits for device to finish current operation if there is any, next it releases all buffers. > And what happens if the app sets the EOS flag, and then later queues another > buffer without that flag. Is that frame accepted/rejected/ignored? I have not take care of this situation. The simplest solution is to reject frames, application in that case should reopen device to encode next stream if necessary. Other solution I see is to allow queue output frames but do not process them by device until device finish producing encoded frames, it would require device reinitialization. > > I'm trying to understand how the current implementation behaves in corner cases > like those. > > > The drawback of this solution is that it wastes resources/space > > (additional buffer) and time (delayed encoding). > > > > I am still hesitating which solution is better, any advices? > > > > > > > > and it should be set only on V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffers. > > > > > > Why only for this type? > > > > I wanted to say only for output buffers not just output multi-plane. And > > why not capture? Explanation below. > > Capture buffers are filled by driver, so only drivers could set this > > flag. Some devices provides information about the end of the stream > > together with the last frame, but some devices provides this info later > > (for example s5p-mfc :) ). In the latter case to properly flag the > > capture buffer driver should wait for next available frame. Simpler > > solution is to use current solution with sending empty buffer to signal > > the end of the stream. > > I don't believe this is documented anywhere. Wouldn't it be better to send > a V4L2_EVENT_EOS event? That's documented and is the way I would expect this > to work. OK, I will change the code accordingly. Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
