linux-amlogic.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Maxime Jourdan <mjourdan@baylibre.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Neil Armstrong <narmstrong@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Kevin Hilman <khilman@baylibre.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hans Verkuil <hans.verkuil@cisco.com>,
	linux-amlogic@lists.infradead.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH v6 3/4] media: meson: add v4l2 m2m video decoder driver
Date: Mon, 27 May 2019 17:41:14 +0200	[thread overview]
Message-ID: <CAMO6nayi+wWU5jqtWkY0riJc6emHiPh7eqpvdzP=U7NgewfwqA@mail.gmail.com> (raw)
In-Reply-To: <0821bfd9-58e4-5df3-4528-189476d35d89@xs4all.nl>

On Mon, May 27, 2019 at 4:54 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 5/27/19 4:44 PM, Maxime Jourdan wrote:
> > Hi Hans,
> > On Mon, May 27, 2019 at 12:04 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> Hi Maxime,
> >>
> >> First a high-level comment: I think this driver should go to staging.
> >> Once we finalize the stateful decoder spec, and we've updated the
> >> v4l2-compliance test, then this needs to be tested against that and
> >> only if it passes can it be moved out of staging.
> >>
> >
> > I chose to send the driver supporting only MPEG2 for now as it keeps
> > it "to the point", but as it turns out it's one of the few formats on
> > Amlogic that can't fully respect the spec at the moment because of the
> > lack of support for V4L2_EVENT_SOURCE_CHANGE, thus the patch in the
> > series that adds a new flag V4L2_FMT_FLAG_FIXED_RESOLUTION. It
> > basically requires userspace to set the format (i.e coded resolution)
> > since the driver/fw can't probe it.
> > At the moment, this is described in the v3 spec like this:
> >
> >>
> >> 1. Set the coded format on ``OUTPUT`` via :c:func:`VIDIOC_S_FMT`
> >>
> >>   * **Required fields:**
> >>
> >>     ``type``
> >>         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``
> >>
> >>     ``pixelformat``
> >>         a coded pixel format
> >>
> >>     ``width``, ``height``
> >>         required only if cannot be parsed from the stream for the given
> >>         coded format; optional otherwise - set to zero to ignore
> >>
> >
> > But MPEG2 being a format where the coded resolution is inside the
> > bitstream, this is purely an Amlogic issue where the firmware doesn't
> > extend the capability to do this.
> >
> > Here's a proposal: if I were to resend the driver supporting only H264
> > and conforming to the spec, would you be considering it for inclusion
> > in the main tree ? Does your current iteration of v4l2-compliance
> > support testing stateful decoders with H264 bitstreams ?
>
> The core problem is that the spec isn't finalized yet. The v3 spec you
> refer to above is old already since there are various changes planned.
>
> If you want to test your driver with a v4l2-compliance that is likely
> to be close to the final version of the spec, then you can use this
> branch:
>
> https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=vicodec
>
> You can test with:
>
> v4l2-compliance -s --stream-from <file>
>
> I wouldn't be too worried about keeping it in staging. Having it there
> will already be very nice indeed. Just add a TODO file that states that
> you are waiting for the final version of the stateful decoder spec and
> the corresponding compliance tests.
>
> The V4L2_FMT_FLAG_FIXED_RESOLUTION isn't a blocker. That flag makes sense,
> and so it has nothing to do with keeping this driver in staging.
>

Okay, I understand. I will send a v7 with the driver in
staging+TODO+MAINTAINERS update.

Regards,
Maxime

> Regards,
>
>         Hans
>
<snip>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2019-05-27 15:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-14 13:56 [PATCH v6 0/4] Add Amlogic video decoder driver Maxime Jourdan
2019-05-14 13:56 ` [PATCH v6 1/4] dt-bindings: media: add Amlogic Video Decoder Bindings Maxime Jourdan
2019-05-14 18:46   ` Martin Blumenstingl
2019-05-14 13:56 ` [PATCH v6 2/4] media: videodev2: add V4L2_FMT_FLAG_FIXED_RESOLUTION Maxime Jourdan
2019-05-14 13:56 ` [PATCH v6 3/4] media: meson: add v4l2 m2m video decoder driver Maxime Jourdan
2019-05-27 10:04   ` Hans Verkuil
2019-05-27 10:18     ` Neil Armstrong
2019-05-27 10:31       ` Hans Verkuil
2019-05-27 14:44     ` Maxime Jourdan
2019-05-27 14:54       ` Hans Verkuil
2019-05-27 15:41         ` Maxime Jourdan [this message]
2019-05-14 13:56 ` [PATCH v6 4/4] MAINTAINERS: Add meson video decoder Maxime Jourdan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMO6nayi+wWU5jqtWkY0riJc6emHiPh7eqpvdzP=U7NgewfwqA@mail.gmail.com' \
    --to=mjourdan@baylibre.com \
    --cc=hans.verkuil@cisco.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=mchehab@kernel.org \
    --cc=narmstrong@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).