Le dimanche 07 juin 2020 à 16:21 -0400, Nicolas Dufresne a écrit : > Le samedi 06 juin 2020 à 09:46 -0300, Ezequiel Garcia a écrit : > > Hi Jernej, > > > > On Thu, 4 Jun 2020 at 15:55, Jernej Skrabec <jernej.skrabec@xxxxxxxx> wrote: > > > Currently H264 interlaced content it's not properly decoded on Cedrus. > > > There are two reasons for this: > > > 1. slice parameters control doesn't provide enough information > > > 2. bug in frame list construction in Cedrus driver > > > > > > As described in commit message in patch 1, references stored in > > > reference lists should tell if reference targets top or bottom field. > > > However, this information is currently not provided. Patch 1 adds > > > it in form of flags which are set for each reference. Patch 2 then > > > uses those flags in Cedrus driver. > > > > > > Frame list construction is fixed in patch 3. > > > > > > This solution was extensively tested using Kodi on LibreELEC with A64, > > > H3, H5 and H6 SoCs in slightly different form (flags were transmitted > > > in MSB bits in index). > > > > > > > So, if I understand correctly the field needs to be passed per-reference, > > and the current per-DPB entry is not good? > > For interlaced content we reference fields separately. That's the > reason there is 32 entries in l0/l1. Though, as we decode both fields > in the same buffer (interleaved), the DPB indice is not sufficient to > inform the decoder what we are referencing. Understand that a top field > can be used to decode the next bottom field. This even make sense as > they are closer on the capture timeline. This covers slice based > decoders. > > The flags to indicate presence of top/bottom fields in DPB array seems > only useful to frame base decoders. This is so that decoder can avoid > using lost fields when constructing it's own l0/l1 internally. > > > If you could point at the userspace code for this, it would be interesting > > to take a look. > > > > > Note: I'm not 100% sure if flags for both, top and bottom fields are > > > needed. Any input here would be welcome. > > > > > > > Given enum v4l2_field is already part of the uAPI, perhaps it makes > > sense to just reuse that for the field type? Maybe it's an overkill, > > but it would make sense to reuse the concepts and types that > > already exist. > > > > We can still add a reserved field to make this new reference type > > extensive. > > I think it's fine to create new flag for this, as your solution would > require extending a reference to 24bit (this patch extend to 16bits). > Though indeed, we could combine frame and TOP and reserve more bits for > future use. Sorry for the oversight, the flags is 16bits, so we already use 24bits. But looking at "enum v4l2_field", which is not a flag, seems like a miss-fit. It would create such a confusion, as V4L2_FIELD_SEQ_TB/BT can still be used with this interface, even though we still need to say if we reference TOP or BOTTOM. Only V4L2_FIELD_ALTERNATE is not supported. But as you can see, "enum v4l2_fiel" is really meant to describe the layout of the interleaved frame rather then signalling a field directly. > > > Thanks, > > Ezequiel > > > > > > > Please take a look. > > > > > > Best regards, > > > Jernej > > > > > > Jernej Skrabec (3): > > > media: uapi: h264: update reference lists > > > media: cedrus: h264: Properly configure reference field > > > media: cedrus: h264: Fix frame list construction > > > > > > .../media/v4l/ext-ctrls-codec.rst | 40 ++++++++++++++++++- > > > .../staging/media/sunxi/cedrus/cedrus_h264.c | 27 +++++++------ > > > include/media/h264-ctrls.h | 12 +++++- > > > 3 files changed, 62 insertions(+), 17 deletions(-) > > > > > > -- > > > 2.27.0 > > > > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
