Hi,
On 11/04/2019 11:18, Hans Verkuil wrote:
On 4/11/19 10:22 AM, Philipp Zabel wrote:
On Wed, 2019-04-10 at 18:24 +0200, Hans Verkuil wrote:
On 4/10/19 6:11 PM, Nicolas Dufresne wrote:
Le mercredi 10 avril 2019 à 16:22 +0200, Philipp Zabel a écrit :
On Wed, 2019-04-10 at 15:43 +0200, Hans Verkuil wrote:
[...]
@@ -1126,12 +1127,32 @@ static int coda_enum_frameintervals(struct file *file, void *fh,
[...]
Why support VIDIOC_ENUM_FRAMEINTERVALS at all? It makes no sense for a codec.
I'd remove it altogether.
It does make sense, since framerate is the only information that can be
used to produce a specific bitrate. If you don't enumerate the rates,
then you may endup with a miss-match of what userspace wants, which
will result in a different rate then what the user-space anticipated.
That being said, I expect these intervals to be really wide. Venus HW
uses a Q16 internally, which is precise enough that we could just
ignore the interval.
So the problem is that for an encoder where you desires a specific
constant bitrate, the encoder also needs to know the framerate.
And the driver then would have to support ENUM_FRAMEINTERVALS and the
S_PARM ioctl so userspace can set the framerate.
Ah right, that was it. Encoding works fine without ENUM_FRAMEINTERVALS,
as coda has no meaningful frame rate limitations. I had implemented
S_PARM since it is required for CBR encoding, and v4l2-compliance then
complained about ENUM_FRAMEINTERVALS missing:
cde29ef313de ("[media] coda: Use S_PARM to set nominal framerate for h.264 encoder")
07b6080d4e6d ("media: coda: implement ENUM_FRAMEINTERVALS")
And I think that's right. If you advertise framerate support, then
userspace needs to know about the capabilities.
I do think ENUM_FRAMEINTERVALS should return some sane values. Right now
coda advertises 1/65535 fps to 65535 fps (or thereabout).
I would probably return 1 fps to 200 fps (200 Hz is the highest framerate
defined by CTA-861-G).
This can also eventually be a helper function in v4l2-mem2mem.c since I expect
this to be the same for all (?) encoders.
While an i.MX wouldn't be my 1st choice for encoding a high framerate
video, it could probably be done. This is a value for the bitrate
control algo, afaics, not a statement of real time performance.
If an arbitrary limit is imposed then userspace would just need to work
around it by adjusting the target bitrate for corner cases like this.
Regards,
Ian
For decoders this would not make any sense AFAIKS, so this is encoder
specific.
Hmm, not necesarily. I hadn't thought about that before, but if the
decoder supports frame skipping this could be used to advertise
available reductions in frame rate.
That assumes that you want to use S_PARM for that as well. And does the decoder
provide the framerate encoded in the bitstream to the driver? Using S_PARM for
this only works if the driver can know the encoded framerate.
Anyway, this is probably something to discuss once a driver supports frame
skipping for the decoder.
Do I understand this correctly?
What should the default framerate be? 24 or 29.97 fps?
Driver specific? Max nominal real time capable frame rate? 30 fps?
This should be documented in the stateful encoder spec. I never realized
that this would be needed...
Philipp, based on this information I would say that ENUM_FRAMEINTERVALS
can stay in the coda drivers, but for encoders only.
Ok.
regards
Philipp
Regards,
Hans
