On Mon, May 27, 2019 at 4:54 PM Hans Verkuil <hverkuil@xxxxxxxxx> 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@xxxxxxxxx> 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>
