On Fri, 5 Jul 2019 19:16:18 +0200 Boris Brezillon <boris.brezil...@collabora.com> wrote:
> On Fri, 05 Jul 2019 13:40:03 -0300 > Ezequiel Garcia <ezequ...@collabora.com> wrote: > > > Hi Boris, Paul, > > > > On Wed, 2019-07-03 at 14:28 +0200, Boris Brezillon wrote: > > > Looks like some stateless decoders expect slices to be prefixed with > > > ANNEX B start codes (they most likely do some kind of bitstream parsing > > > and/or need that to delimit slices when doing per frame decoding). > > > Since skipping those start codes for dummy stateless decoders (those > > > expecting all params to be passed through controls) should be pretty > > > easy, let's mandate that all slices be prepended with ANNEX B start > > > codes. > > > > > > If we ever need to support AVC headers, we can add a new menu control > > > to select the type of NAL header to use. > > > > > > Signed-off-by: Boris Brezillon <boris.brezil...@collabora.com> > > > Reviewed-by: Paul Kocialkowski <paul.kocialkow...@bootlin.com> > > > --- > > > Changes in v3: > > > * Add Paul's R-b > > > > > > Changes in v2: > > > * None > > > --- > > > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > index 7a1947f5be96..3ae1367806cf 100644 > > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > @@ -1726,6 +1726,7 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > > :ref:`h264`, section 7.4.3 "Slice Header Semantics". For further > > > documentation, refer to the above specification, unless there is > > > an explicit comment stating otherwise. > > > + All slices should be prepended with an ANNEX B start code. > > > > > > > Currently, the H264 slice V4L2_PIX_FMT_H264_SLICE_RAW, > > is specified to _not_ contain the ANNEX B start code. > > Yep, we should provably rename the format. Paul, are you okay with this rename? s/V4L2_PIX_FMT_H264_SLICE_RAW/V4L2_PIX_FMT_H264_SLICE/ or s/V4L2_PIX_FMT_H264_SLICE_RAW/V4L2_PIX_FMT_H264_SLICE_ANNEXB/ I'd also to discuss some concerns Ezequiel and I have regarding this change. Some (most?) codec have alignment constraints on the buffer they pass to the HW. For HW that support Annex B parsing, that's no problem because the start of the buffer will be aligned on a page (I'm assuming page alignment should cover 99% of the alignment constraints). But HW that need to skip the start code will have to pass a non-aligned buffer (annex B start code is 3 byte long). Paul looked at the Cedrus driver and thinks it can be handled correctly thanks to the VE_H264_VLD_OFFSET field (which encodes an offset in bit), but I fear this might be a problem on other HW. So, I'm asking again, are we sure we want to handle the raw (no start code) and annex-b cases using the same pixel format? If we do, what's the plan to address those potential alignment constraints? Should we provide a way for userspace to define where the start-code ends so it can align things properly (annex B can be extended with extra 00 bytes at the beginning)? If we do that, that means userspace has to know about those alignment constraints, or take something big enough. Another option would be to use a bounce buffer when things are not aligned properly. I'd really like to get feedback on those points before sending a v4.
