New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 639238 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Improve H264 stream header handling in V4L2VEA

Project Member Reported by posciak@chromium.org, Aug 19 2016

Issue description

Currently, V4L2VEA assumes that the first H264 bitstream buffer coming from the hardware encoder will contain exactly an SPS+PPS pair. It then caches that buffer and prepends its contents before each keyframe. It also uses it for the remainder of the session, even if additional/new SPS+PPS pairs are provided later on.

However, the first buffer may contain additional NALUs, and only SPS+PPS should be extracted and used. Moreover, the SPS+PPS pair may change later on. Finally, some hardware encoders may support prepending each IDR with the header, without the need for V4L2VEA to handle this.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 24 2016

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0bcdec29b2e03277c81f566fe96fd7ef62f54b17

commit 0bcdec29b2e03277c81f566fe96fd7ef62f54b17
Author: Pawel Osciak <posciak@chromium.org>
Date: Fri Aug 19 09:12:09 2016

CHROMIUM: v4l: Add V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR

When set, the V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR control requests
an H.264 encoder to insert an SPS and PPS pair before each IDR.

Signed-off-by: Pawel Osciak <posciak@chromium.org>

BUG= chromium:639238 
TEST=veaunittest

Change-Id: I17b41a8a3e228338f5c59dcdfb38ca04cfa5119b
Reviewed-on: https://chromium-review.googlesource.com/373286
Commit-Ready: Pawel Osciak <posciak@chromium.org>
Tested-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/0bcdec29b2e03277c81f566fe96fd7ef62f54b17/include/uapi/linux/v4l2-controls.h
[modify] https://crrev.com/0bcdec29b2e03277c81f566fe96fd7ef62f54b17/drivers/media/v4l2-core/v4l2-ctrls.c

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db74e2bf3a0bb5a7b4e2c4ed6b14e75ffbcffcc3

commit db74e2bf3a0bb5a7b4e2c4ed6b14e75ffbcffcc3
Author: posciak <posciak@chromium.org>
Date: Thu Aug 25 05:47:27 2016

V4L2VEA: Improve H264 stream header handling.

Currently, V4L2VEA assumes that the first H264 bitstream buffer coming from
the hardware encoder will contain exactly an SPS+PPS pair. It then caches
that buffer and prepends its contents before each keyframe. It also uses it
for the remainder of the session, even if additional/new SPS+PPS pairs are
provided later on.

However, the first buffer may contain additional NALUs, and only SPS+PPS
should be extracted and used. Moreover, the SPS+PPS pair may change later
on. Finally, some hardware encoders may support prepending each IDR with
the header, without the need for V4L2VEA to handle this.

This CL adds support for the V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR
V4L2 control, which instructs the HW encoder to inject SPS+PPS pair before
each IDR in the stream. If that control is supported, we do not have to
inject the header ourselves.

If the control is not supported, we need to keep injecting, however -
instead of making the above assumptions - we parse the stream, cache the
latest SPS and PPS, and inject them before any found IDR, if possible.

The VEA unittest is also modified, to verify that the stream includes an
SPS+PPS pair before each IDR.

Also, remove usage of linked_ptr for clearer ownership management of
buffers.

BUG= 639238 
TEST=veatest on various ARM platforms

Review-Url: https://codereview.chromium.org/2274493002
Cr-Commit-Position: refs/heads/master@{#414332}

[modify] https://crrev.com/db74e2bf3a0bb5a7b4e2c4ed6b14e75ffbcffcc3/media/gpu/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/db74e2bf3a0bb5a7b4e2c4ed6b14e75ffbcffcc3/media/gpu/v4l2_video_encode_accelerator.h
[modify] https://crrev.com/db74e2bf3a0bb5a7b4e2c4ed6b14e75ffbcffcc3/media/gpu/video_encode_accelerator_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 25 2016

Labels: merge-merged-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f0866f934010b1ac54bd6b680633905d10b29d52

commit f0866f934010b1ac54bd6b680633905d10b29d52
Author: Pawel Osciak <posciak@chromium.org>
Date: Fri Aug 19 09:12:09 2016

CHROMIUM: v4l: Add V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR

When set, the V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR control requests
an H.264 encoder to insert an SPS and PPS pair before each IDR.

Signed-off-by: Pawel Osciak <posciak@chromium.org>

BUG= chromium:639238 
TEST=veaunittest

Change-Id: I17b41a8a3e228338f5c59dcdfb38ca04cfa5119b
Reviewed-on: https://chromium-review.googlesource.com/373259
Commit-Ready: Pawel Osciak <posciak@chromium.org>
Tested-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/f0866f934010b1ac54bd6b680633905d10b29d52/include/uapi/linux/v4l2-controls.h
[modify] https://crrev.com/f0866f934010b1ac54bd6b680633905d10b29d52/drivers/media/v4l2-core/v4l2-ctrls.c

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b2a7593c1e49fee7f929f56347ed36bab72b78d8

commit b2a7593c1e49fee7f929f56347ed36bab72b78d8
Author: Pawel Osciak <posciak@chromium.org>
Date: Fri Aug 19 10:00:08 2016

CHROMIUM: s5p-mfc: Improve h264_options register handling.

Don't overwrite previous settings with subsequent reads and writes to
the h264_options register. Also, coalesce reads and writes in sequence.

Signed-off-by: Pawel Osciak <posciak@chromium.org>

BUG= chromium:639238 
TEST=veaunittest

Change-Id: I78d21cca4972c262b8795210bbc8ee04c1e34274
Reviewed-on: https://chromium-review.googlesource.com/373260
Commit-Ready: Pawel Osciak <posciak@chromium.org>
Tested-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/b2a7593c1e49fee7f929f56347ed36bab72b78d8/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c3b27303808f10ea91bb7ae191e6862226d4ec6c

commit c3b27303808f10ea91bb7ae191e6862226d4ec6c
Author: Pawel Osciak <posciak@chromium.org>
Date: Fri Aug 19 10:14:45 2016

CHROMIUM: s5p-mfc: Support V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR

Add support for V4L2_CID_MPEG_VIDEO_H264_SPS_PPS_BEFORE_IDR control to
instruct the H.264 encoder to prepend each IDR with SPS and PPS.

Signed-off-by: Pawel Osciak <posciak@chromium.org>

BUG= chromium:639238 
TEST=veatest

Change-Id: I52f2765e2659c51ceb6a3aef48087648530d072d
Reviewed-on: https://chromium-review.googlesource.com/373261
Commit-Ready: Pawel Osciak <posciak@chromium.org>
Tested-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/c3b27303808f10ea91bb7ae191e6862226d4ec6c/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
[modify] https://crrev.com/c3b27303808f10ea91bb7ae191e6862226d4ec6c/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
[modify] https://crrev.com/c3b27303808f10ea91bb7ae191e6862226d4ec6c/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c

Sign in to add a comment