Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200604185745.23568-1-jernej.skrabec@siol.net>
In-Reply-To: <20200604185745.23568-1-jernej.skrabec@siol.net>
From:   Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Date:   Sat, 6 Jun 2020 09:46:10 -0300
Message-ID: <CAAEAJfDFMzMkDkN7zzNvkwsmYzgQPNGkP=dhW7neycYYRBJzHA@mail.gmail.com>
Subject: Re: [PATCH 0/3] media: uapi: cedrus: Fix decoding interlaced H264 content
To:     Jernej Skrabec <jernej.skrabec@siol.net>
Cc:     Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
        Maxime Ripard <mripard@kernel.org>, devel@driverdev.osuosl.org,
        Jonas Karlman <jonas@kwiboo.se>,
        Greg KH <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Nicolas Dufresne <nicolas@ndufresne.ca>,
        Chen-Yu Tsai <wens@csie.org>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-media <linux-media@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jernej,

On Thu, 4 Jun 2020 at 15:55, Jernej Skrabec <jernej.skrabec@siol.net> 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?

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.

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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel