Message ID | 20191122051608.128717-1-hiroh@chromium.org (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Hans Verkuil |
Headers |
Received: from vger.kernel.org ([209.132.180.67]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1iY1JA-000jBj-Vd; Fri, 22 Nov 2019 05:16:41 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726634AbfKVFQU (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 22 Nov 2019 00:16:20 -0500 Received: from mail-pf1-f196.google.com ([209.85.210.196]:45142 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726018AbfKVFQU (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 22 Nov 2019 00:16:20 -0500 Received: by mail-pf1-f196.google.com with SMTP id z4so2893683pfn.12 for <linux-media@vger.kernel.org>; Thu, 21 Nov 2019 21:16:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Pur8D56rYiQ0M1ceqnI28F7eCh0YYOJc2gZei1cSwYw=; b=QUuL2c+xQxdrPyysEXsec8QavmH64GVr/WK5Xy2ffOtjwjSkRDU8nM231KQZ55gTT+ o95OPytdoXojkQe06wVttIqQ+d8ikveWepYkDjh3szE/vnrqQQwCVhIHbokE9ohTbJBr Qir9fZmE4Y8oLlw+ShfjvkYD9U9A5TCKfg4Jw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Pur8D56rYiQ0M1ceqnI28F7eCh0YYOJc2gZei1cSwYw=; b=WtUADStOXzOdQ3DuiP2EO+HAgpKgnnu2p02CoVDrmXmpRxdYtrk8HKyWoC3FhKH1/N UkUjyIleb/79OQTvReCnz5iSy1uZP89HGKsZJ72bT2WscQABUh0u2D6Cr20HpT9TpZl5 8pjPkrNEbutGl0QQXDhjyohFYl2M1wgnFIk4lQAe0tmk88t+zXcrQ+78dRVVzfHsrC/c oXXwR/6YGf8Y6RDvjt4p8s+XBhDqtf/IWRw9YaomJP0K4fmf1E+N9kFL73AYw5HbNXgB 0G1RRsgzkE9lgHVGkEPU4uarYHjEVlYgi8nPWhKBuMPjHSF2ywuUbNQ4xePDHuwcagDO OgGQ== X-Gm-Message-State: APjAAAUN6KquYDGDd4lxj8SlXTaU51+6ygYNErHhTQXwvA+zkCFk6eVh UbcEEfgFmrakRY+CUHTRWZzqhw== X-Google-Smtp-Source: APXvYqwm6Bo6Fpvepq74mr6KZ7xSHs2WiOshfDeBDm7ixVWj28grrqSlnrpqRI/kf/OPwZ6eICDjRg== X-Received: by 2002:a63:fa0d:: with SMTP id y13mr13431599pgh.18.1574399777846; Thu, 21 Nov 2019 21:16:17 -0800 (PST) Received: from hiroh.tok.corp.google.com ([2401:fa00:8f:2:3bf4:6372:5b56:dd4c]) by smtp.gmail.com with ESMTPSA id m7sm5756049pfb.153.2019.11.21.21.16.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Nov 2019 21:16:17 -0800 (PST) From: Hirokazu Honda <hiroh@chromium.org> To: ezequiel@collabora.com, mchehab@kernel.org, gregkh@linuxfoundation.org, linux-media@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, tfiga@chromium.org Cc: Hirokazu Honda <hiroh@chromium.org> Subject: [PATCH] media: hantro: Support H264 profile control Date: Fri, 22 Nov 2019 14:16:08 +0900 Message-Id: <20191122051608.128717-1-hiroh@chromium.org> X-Mailer: git-send-email 2.24.0.432.g9d3f5f5b63-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Series |
media: hantro: Support H264 profile control
|
|
Commit Message
Hirokazu Honda
Nov. 22, 2019, 5:16 a.m. UTC
The Hantro G1 decoder supports H.264 profiles from Baseline to High, with
the exception of the Extended profile.
Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the
applications can query the driver for the list of supported profiles.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++
1 file changed, 10 insertions(+)
Comments
Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > The Hantro G1 decoder supports H.264 profiles from Baseline to High, with > the exception of the Extended profile. > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > applications can query the driver for the list of supported profiles. Thanks for this patch. Do you think we could also add the LEVEL control so the profile/level enumeration becomes complete ? I'm thinking it would be nice if the v4l2 compliance test make sure that codecs do implement these controls (both stateful and stateless), it's essential for stack with software fallback, or multiple capable codec hardware but with different capabilities. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > index 6d9d41170832..9387619235d8 100644 > --- a/drivers/staging/media/hantro/hantro_drv.c > +++ b/drivers/staging/media/hantro/hantro_drv.c > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > }, > + }, { > + .codec = HANTRO_H264_DECODER, > + .cfg = { > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > + .menu_skip_mask = > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > + } > }, { > }, > };
On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, with > > the exception of the Extended profile. > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > applications can query the driver for the list of supported profiles. > > Thanks for this patch. Do you think we could also add the LEVEL control > so the profile/level enumeration becomes complete ? > > I'm thinking it would be nice if the v4l2 compliance test make sure > that codecs do implement these controls (both stateful and stateless), > it's essential for stack with software fallback, or multiple capable > codec hardware but with different capabilities. > Level is a difficult story, because it also specifies the number of macroblocks per second, but for decoders like this the number of macroblocks per second it can handle depends on things the driver might be not aware of - clock frequencies, DDR throughput, system load, etc. My take on this is that the decoder driver should advertise the highest resolution the decoder can handle due to hardware constraints. Performance related things depend on the integration details and should be managed elsewhere. For example Android and Chrome OS manage expected decoding performance in per-board configuration files. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > --- > > drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > > index 6d9d41170832..9387619235d8 100644 > > --- a/drivers/staging/media/hantro/hantro_drv.c > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > > .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > }, > > + }, { > > + .codec = HANTRO_H264_DECODER, > > + .cfg = { > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > + .menu_skip_mask = > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > > + } > > }, { > > }, > > }; >
Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, with > > > the exception of the Extended profile. > > > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > > applications can query the driver for the list of supported profiles. > > > > Thanks for this patch. Do you think we could also add the LEVEL control > > so the profile/level enumeration becomes complete ? > > > > I'm thinking it would be nice if the v4l2 compliance test make sure > > that codecs do implement these controls (both stateful and stateless), > > it's essential for stack with software fallback, or multiple capable > > codec hardware but with different capabilities. > > > > Level is a difficult story, because it also specifies the number of > macroblocks per second, but for decoders like this the number of > macroblocks per second it can handle depends on things the driver > might be not aware of - clock frequencies, DDR throughput, system > load, etc. > > My take on this is that the decoder driver should advertise the > highest resolution the decoder can handle due to hardware constraints. > Performance related things depend on the integration details and > should be managed elsewhere. For example Android and Chrome OS manage > expected decoding performance in per-board configuration files. When you read datasheet, the HW is always rated to maximum level (and it's a range) with the assumption of a single stream. It seems much easier to expose this as-is, statically then to start doing some math with data that isn't fully exposed to the user. This is about filtering of multiple CODEC instances, it does not need to be rocket science, specially that the amount of missing data is important (e.g. usage of tiles, compression, IPP all have an impact on the final performance). All we want to know in userspace is if this HW is even possibly capable of LEVEL X, and if not, we'll look for another one. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > --- > > > drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > > > index 6d9d41170832..9387619235d8 100644 > > > --- a/drivers/staging/media/hantro/hantro_drv.c > > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > > > .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > }, > > > + }, { > > > + .codec = HANTRO_H264_DECODER, > > > + .cfg = { > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > > + .menu_skip_mask = > > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > > > + } > > > }, { > > > }, > > > };
On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > > On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, with > > > > the exception of the Extended profile. > > > > > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > > > applications can query the driver for the list of supported profiles. > > > > > > Thanks for this patch. Do you think we could also add the LEVEL control > > > so the profile/level enumeration becomes complete ? > > > > > > I'm thinking it would be nice if the v4l2 compliance test make sure > > > that codecs do implement these controls (both stateful and stateless), > > > it's essential for stack with software fallback, or multiple capable > > > codec hardware but with different capabilities. > > > > > > > Level is a difficult story, because it also specifies the number of > > macroblocks per second, but for decoders like this the number of > > macroblocks per second it can handle depends on things the driver > > might be not aware of - clock frequencies, DDR throughput, system > > load, etc. > > > > My take on this is that the decoder driver should advertise the > > highest resolution the decoder can handle due to hardware constraints. > > Performance related things depend on the integration details and > > should be managed elsewhere. For example Android and Chrome OS manage > > expected decoding performance in per-board configuration files. > > When you read datasheet, the HW is always rated to maximum level (and > it's a range) with the assumption of a single stream. It seems much > easier to expose this as-is, statically then to start doing some math > with data that isn't fully exposed to the user. This is about filtering > of multiple CODEC instances, it does not need to be rocket science, > specially that the amount of missing data is important (e.g. usage of > tiles, compression, IPP all have an impact on the final performance). > All we want to know in userspace is if this HW is even possibly capable > of LEVEL X, and if not, we'll look for another one. > Agreed, one could potentially define it this way, but would it be really useful for the userspace and the users? I guess it could enable slightly faster fallback to software decoding in the extreme case of the hardware not supporting the level at all, but I suspect that the majority of cases would be the hardware just being unusably slow. Also, as I mentioned before, we already return the range of supported resolutions, which in practice should map to the part of the level that may depend on hardware capabilities rather than performance, so exposing levels as well would add redundancy to the information exposed. > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > --- > > > > drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > > > > index 6d9d41170832..9387619235d8 100644 > > > > --- a/drivers/staging/media/hantro/hantro_drv.c > > > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > > > > .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > }, > > > > + }, { > > > > + .codec = HANTRO_H264_DECODER, > > > > + .cfg = { > > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > > > + .menu_skip_mask = > > > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > > > > + } > > > > }, { > > > > }, > > > > };
On 11/29/19 1:16 AM, Tomasz Figa wrote: > On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: >> >> Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : >>> On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: >>>> Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : >>>>> The Hantro G1 decoder supports H.264 profiles from Baseline to High, with >>>>> the exception of the Extended profile. >>>>> >>>>> Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the >>>>> applications can query the driver for the list of supported profiles. >>>> >>>> Thanks for this patch. Do you think we could also add the LEVEL control >>>> so the profile/level enumeration becomes complete ? >>>> >>>> I'm thinking it would be nice if the v4l2 compliance test make sure >>>> that codecs do implement these controls (both stateful and stateless), >>>> it's essential for stack with software fallback, or multiple capable >>>> codec hardware but with different capabilities. >>>> >>> >>> Level is a difficult story, because it also specifies the number of >>> macroblocks per second, but for decoders like this the number of >>> macroblocks per second it can handle depends on things the driver >>> might be not aware of - clock frequencies, DDR throughput, system >>> load, etc. >>> >>> My take on this is that the decoder driver should advertise the >>> highest resolution the decoder can handle due to hardware constraints. >>> Performance related things depend on the integration details and >>> should be managed elsewhere. For example Android and Chrome OS manage >>> expected decoding performance in per-board configuration files. >> >> When you read datasheet, the HW is always rated to maximum level (and >> it's a range) with the assumption of a single stream. It seems much >> easier to expose this as-is, statically then to start doing some math >> with data that isn't fully exposed to the user. This is about filtering >> of multiple CODEC instances, it does not need to be rocket science, >> specially that the amount of missing data is important (e.g. usage of >> tiles, compression, IPP all have an impact on the final performance). >> All we want to know in userspace is if this HW is even possibly capable >> of LEVEL X, and if not, we'll look for another one. >> > > Agreed, one could potentially define it this way, but would it be > really useful for the userspace and the users? I guess it could enable > slightly faster fallback to software decoding in the extreme case of > the hardware not supporting the level at all, but I suspect that the > majority of cases would be the hardware just being unusably slow. > > Also, as I mentioned before, we already return the range of supported > resolutions, which in practice should map to the part of the level > that may depend on hardware capabilities rather than performance, so > exposing levels as well would add redundancy to the information > exposed. There is a lot of discussion here, but it all revolves around a potential LEVEL control. So I gather everyone is OK with merging this patch? If not, let me know asap. Regards, Hans > >>> >>>>> Signed-off-by: Hirokazu Honda <hiroh@chromium.org> >>>>> --- >>>>> drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ >>>>> 1 file changed, 10 insertions(+) >>>>> >>>>> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c >>>>> index 6d9d41170832..9387619235d8 100644 >>>>> --- a/drivers/staging/media/hantro/hantro_drv.c >>>>> +++ b/drivers/staging/media/hantro/hantro_drv.c >>>>> @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { >>>>> .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, >>>>> .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, >>>>> }, >>>>> + }, { >>>>> + .codec = HANTRO_H264_DECODER, >>>>> + .cfg = { >>>>> + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, >>>>> + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, >>>>> + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, >>>>> + .menu_skip_mask = >>>>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), >>>>> + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, >>>>> + } >>>>> }, { >>>>> }, >>>>> };
Hello Hirokazu, Hans, On Fri, 2020-01-10 at 13:31 +0100, Hans Verkuil wrote: > On 11/29/19 1:16 AM, Tomasz Figa wrote: > > On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > > Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > > > > On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > > > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > > > > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, with > > > > > > the exception of the Extended profile. > > > > > > > > > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > > > > > applications can query the driver for the list of supported profiles. > > > > > > > > > > Thanks for this patch. Do you think we could also add the LEVEL control > > > > > so the profile/level enumeration becomes complete ? > > > > > > > > > > I'm thinking it would be nice if the v4l2 compliance test make sure > > > > > that codecs do implement these controls (both stateful and stateless), > > > > > it's essential for stack with software fallback, or multiple capable > > > > > codec hardware but with different capabilities. > > > > > > > > > > > > > Level is a difficult story, because it also specifies the number of > > > > macroblocks per second, but for decoders like this the number of > > > > macroblocks per second it can handle depends on things the driver > > > > might be not aware of - clock frequencies, DDR throughput, system > > > > load, etc. > > > > > > > > My take on this is that the decoder driver should advertise the > > > > highest resolution the decoder can handle due to hardware constraints. > > > > Performance related things depend on the integration details and > > > > should be managed elsewhere. For example Android and Chrome OS manage > > > > expected decoding performance in per-board configuration files. > > > > > > When you read datasheet, the HW is always rated to maximum level (and > > > it's a range) with the assumption of a single stream. It seems much > > > easier to expose this as-is, statically then to start doing some math > > > with data that isn't fully exposed to the user. This is about filtering > > > of multiple CODEC instances, it does not need to be rocket science, > > > specially that the amount of missing data is important (e.g. usage of > > > tiles, compression, IPP all have an impact on the final performance). > > > All we want to know in userspace is if this HW is even possibly capable > > > of LEVEL X, and if not, we'll look for another one. > > > > > > > Agreed, one could potentially define it this way, but would it be > > really useful for the userspace and the users? I guess it could enable > > slightly faster fallback to software decoding in the extreme case of > > the hardware not supporting the level at all, but I suspect that the > > majority of cases would be the hardware just being unusably slow. > > > > Also, as I mentioned before, we already return the range of supported > > resolutions, which in practice should map to the part of the level > > that may depend on hardware capabilities rather than performance, so > > exposing levels as well would add redundancy to the information > > exposed. > > There is a lot of discussion here, but it all revolves around a potential > LEVEL control. > > So I gather everyone is OK with merging this patch? > I'm fine with this. [..] > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > > > > > > index 6d9d41170832..9387619235d8 100644 > > > > > > --- a/drivers/staging/media/hantro/hantro_drv.c > > > > > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > > > > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > > > > > > .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > > > .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > > > }, > > > > > > + }, { > > > > > > + .codec = HANTRO_H264_DECODER, > > > > > > + .cfg = { > > > > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > > > > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, I'd like to see the .def here ^^^ for consistency with the other controls. But I know this is my OCD speaking, so it's fine as-is as well. Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com> > > > > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > > > > > + .menu_skip_mask = > > > > > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > > > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, Thanks, Eze
Le vendredi 10 janvier 2020 à 13:31 +0100, Hans Verkuil a écrit : > On 11/29/19 1:16 AM, Tomasz Figa wrote: > > On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> > > wrote: > > > Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > > > > On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> > > > > wrote: > > > > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > > > > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, > > > > > > with > > > > > > the exception of the Extended profile. > > > > > > > > > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > > > > > applications can query the driver for the list of supported > > > > > > profiles. > > > > > > > > > > Thanks for this patch. Do you think we could also add the LEVEL > > > > > control > > > > > so the profile/level enumeration becomes complete ? > > > > > > > > > > I'm thinking it would be nice if the v4l2 compliance test make sure > > > > > that codecs do implement these controls (both stateful and stateless), > > > > > it's essential for stack with software fallback, or multiple capable > > > > > codec hardware but with different capabilities. > > > > > > > > > > > > > Level is a difficult story, because it also specifies the number of > > > > macroblocks per second, but for decoders like this the number of > > > > macroblocks per second it can handle depends on things the driver > > > > might be not aware of - clock frequencies, DDR throughput, system > > > > load, etc. > > > > > > > > My take on this is that the decoder driver should advertise the > > > > highest resolution the decoder can handle due to hardware constraints. > > > > Performance related things depend on the integration details and > > > > should be managed elsewhere. For example Android and Chrome OS manage > > > > expected decoding performance in per-board configuration files. > > > > > > When you read datasheet, the HW is always rated to maximum level (and > > > it's a range) with the assumption of a single stream. It seems much > > > easier to expose this as-is, statically then to start doing some math > > > with data that isn't fully exposed to the user. This is about filtering > > > of multiple CODEC instances, it does not need to be rocket science, > > > specially that the amount of missing data is important (e.g. usage of > > > tiles, compression, IPP all have an impact on the final performance). > > > All we want to know in userspace is if this HW is even possibly capable > > > of LEVEL X, and if not, we'll look for another one. > > > > > > > Agreed, one could potentially define it this way, but would it be > > really useful for the userspace and the users? I guess it could enable > > slightly faster fallback to software decoding in the extreme case of > > the hardware not supporting the level at all, but I suspect that the > > majority of cases would be the hardware just being unusably slow. > > > > Also, as I mentioned before, we already return the range of supported > > resolutions, which in practice should map to the part of the level > > that may depend on hardware capabilities rather than performance, so > > exposing levels as well would add redundancy to the information > > exposed. > > There is a lot of discussion here, but it all revolves around a potential > LEVEL control. > > So I gather everyone is OK with merging this patch? I'm ok with this. For me, the level reflects the real time performance capability as define in the spec, while the width/height constraints usually represent an addressing capabicity, which may or may not operate real-time. > > If not, let me know asap. > > Regards, > > Hans > > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > > > --- > > > > > > drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c > > > > > > b/drivers/staging/media/hantro/hantro_drv.c > > > > > > index 6d9d41170832..9387619235d8 100644 > > > > > > --- a/drivers/staging/media/hantro/hantro_drv.c > > > > > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > > > > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > > > > > > .def = > > > > > > V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > > > .max = > > > > > > V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > > > }, > > > > > > + }, { > > > > > > + .codec = HANTRO_H264_DECODER, > > > > > > + .cfg = { > > > > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > > > > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > > > > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > > > > > + .menu_skip_mask = > > > > > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > > > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > > > > > > + } > > > > > > }, { > > > > > > }, > > > > > > };
On Sat, Jan 18, 2020 at 10:31 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > Le vendredi 10 janvier 2020 à 13:31 +0100, Hans Verkuil a écrit : > > On 11/29/19 1:16 AM, Tomasz Figa wrote: > > > On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> > > > wrote: > > > > Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > > > > > On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> > > > > > wrote: > > > > > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > > > > > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, > > > > > > > with > > > > > > > the exception of the Extended profile. > > > > > > > > > > > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > > > > > > applications can query the driver for the list of supported > > > > > > > profiles. > > > > > > > > > > > > Thanks for this patch. Do you think we could also add the LEVEL > > > > > > control > > > > > > so the profile/level enumeration becomes complete ? > > > > > > > > > > > > I'm thinking it would be nice if the v4l2 compliance test make sure > > > > > > that codecs do implement these controls (both stateful and stateless), > > > > > > it's essential for stack with software fallback, or multiple capable > > > > > > codec hardware but with different capabilities. > > > > > > > > > > > > > > > > Level is a difficult story, because it also specifies the number of > > > > > macroblocks per second, but for decoders like this the number of > > > > > macroblocks per second it can handle depends on things the driver > > > > > might be not aware of - clock frequencies, DDR throughput, system > > > > > load, etc. > > > > > > > > > > My take on this is that the decoder driver should advertise the > > > > > highest resolution the decoder can handle due to hardware constraints. > > > > > Performance related things depend on the integration details and > > > > > should be managed elsewhere. For example Android and Chrome OS manage > > > > > expected decoding performance in per-board configuration files. > > > > > > > > When you read datasheet, the HW is always rated to maximum level (and > > > > it's a range) with the assumption of a single stream. It seems much > > > > easier to expose this as-is, statically then to start doing some math > > > > with data that isn't fully exposed to the user. This is about filtering > > > > of multiple CODEC instances, it does not need to be rocket science, > > > > specially that the amount of missing data is important (e.g. usage of > > > > tiles, compression, IPP all have an impact on the final performance). > > > > All we want to know in userspace is if this HW is even possibly capable > > > > of LEVEL X, and if not, we'll look for another one. > > > > > > > > > > Agreed, one could potentially define it this way, but would it be > > > really useful for the userspace and the users? I guess it could enable > > > slightly faster fallback to software decoding in the extreme case of > > > the hardware not supporting the level at all, but I suspect that the > > > majority of cases would be the hardware just being unusably slow. > > > > > > Also, as I mentioned before, we already return the range of supported > > > resolutions, which in practice should map to the part of the level > > > that may depend on hardware capabilities rather than performance, so > > > exposing levels as well would add redundancy to the information > > > exposed. > > > > There is a lot of discussion here, but it all revolves around a potential > > LEVEL control. > > > > So I gather everyone is OK with merging this patch? > > I'm ok with this. For me, the level reflects the real time performance > capability as define in the spec, while the width/height constraints usually > represent an addressing capabicity, which may or may not operate real-time. > I'd like to see the level control documentation improved before we start adding it to the drivers. I'll be okay with that then as long as the values are exposed in a consistent and well defined way. :) As for this patch, Hans, are you going to apply it? Best regards, Tomasz > > > > If not, let me know asap. > > > > Regards, > > > > Hans > > > > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > > > > --- > > > > > > > drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > > > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c > > > > > > > b/drivers/staging/media/hantro/hantro_drv.c > > > > > > > index 6d9d41170832..9387619235d8 100644 > > > > > > > --- a/drivers/staging/media/hantro/hantro_drv.c > > > > > > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > > > > > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > > > > > > > .def = > > > > > > > V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > > > > .max = > > > > > > > V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > > > > > > > }, > > > > > > > + }, { > > > > > > > + .codec = HANTRO_H264_DECODER, > > > > > > > + .cfg = { > > > > > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > > > > > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > > > > > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > > > > > > + .menu_skip_mask = > > > > > > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > > > > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > > > > > > > + } > > > > > > > }, { > > > > > > > }, > > > > > > > }; >
On 2/3/20 12:13 PM, Tomasz Figa wrote: > On Sat, Jan 18, 2020 at 10:31 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: >> >> Le vendredi 10 janvier 2020 à 13:31 +0100, Hans Verkuil a écrit : >>> On 11/29/19 1:16 AM, Tomasz Figa wrote: >>>> On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> >>>> wrote: >>>>> Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : >>>>>> On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> >>>>>> wrote: >>>>>>> Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : >>>>>>>> The Hantro G1 decoder supports H.264 profiles from Baseline to High, >>>>>>>> with >>>>>>>> the exception of the Extended profile. >>>>>>>> >>>>>>>> Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the >>>>>>>> applications can query the driver for the list of supported >>>>>>>> profiles. >>>>>>> >>>>>>> Thanks for this patch. Do you think we could also add the LEVEL >>>>>>> control >>>>>>> so the profile/level enumeration becomes complete ? >>>>>>> >>>>>>> I'm thinking it would be nice if the v4l2 compliance test make sure >>>>>>> that codecs do implement these controls (both stateful and stateless), >>>>>>> it's essential for stack with software fallback, or multiple capable >>>>>>> codec hardware but with different capabilities. >>>>>>> >>>>>> >>>>>> Level is a difficult story, because it also specifies the number of >>>>>> macroblocks per second, but for decoders like this the number of >>>>>> macroblocks per second it can handle depends on things the driver >>>>>> might be not aware of - clock frequencies, DDR throughput, system >>>>>> load, etc. >>>>>> >>>>>> My take on this is that the decoder driver should advertise the >>>>>> highest resolution the decoder can handle due to hardware constraints. >>>>>> Performance related things depend on the integration details and >>>>>> should be managed elsewhere. For example Android and Chrome OS manage >>>>>> expected decoding performance in per-board configuration files. >>>>> >>>>> When you read datasheet, the HW is always rated to maximum level (and >>>>> it's a range) with the assumption of a single stream. It seems much >>>>> easier to expose this as-is, statically then to start doing some math >>>>> with data that isn't fully exposed to the user. This is about filtering >>>>> of multiple CODEC instances, it does not need to be rocket science, >>>>> specially that the amount of missing data is important (e.g. usage of >>>>> tiles, compression, IPP all have an impact on the final performance). >>>>> All we want to know in userspace is if this HW is even possibly capable >>>>> of LEVEL X, and if not, we'll look for another one. >>>>> >>>> >>>> Agreed, one could potentially define it this way, but would it be >>>> really useful for the userspace and the users? I guess it could enable >>>> slightly faster fallback to software decoding in the extreme case of >>>> the hardware not supporting the level at all, but I suspect that the >>>> majority of cases would be the hardware just being unusably slow. >>>> >>>> Also, as I mentioned before, we already return the range of supported >>>> resolutions, which in practice should map to the part of the level >>>> that may depend on hardware capabilities rather than performance, so >>>> exposing levels as well would add redundancy to the information >>>> exposed. >>> >>> There is a lot of discussion here, but it all revolves around a potential >>> LEVEL control. >>> >>> So I gather everyone is OK with merging this patch? >> >> I'm ok with this. For me, the level reflects the real time performance >> capability as define in the spec, while the width/height constraints usually >> represent an addressing capabicity, which may or may not operate real-time. >> > > I'd like to see the level control documentation improved before we > start adding it to the drivers. I'll be okay with that then as long as > the values are exposed in a consistent and well defined way. :) > > As for this patch, Hans, are you going to apply it? It's in a PR for 5.7. I had hoped it would go in for v5.6, but it was too late for that. Regards, Hans > > Best regards, > Tomasz > >>> >>> If not, let me know asap. >>> >>> Regards, >>> >>> Hans >>> >>>>>>>> Signed-off-by: Hirokazu Honda <hiroh@chromium.org> >>>>>>>> --- >>>>>>>> drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ >>>>>>>> 1 file changed, 10 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/staging/media/hantro/hantro_drv.c >>>>>>>> b/drivers/staging/media/hantro/hantro_drv.c >>>>>>>> index 6d9d41170832..9387619235d8 100644 >>>>>>>> --- a/drivers/staging/media/hantro/hantro_drv.c >>>>>>>> +++ b/drivers/staging/media/hantro/hantro_drv.c >>>>>>>> @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { >>>>>>>> .def = >>>>>>>> V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, >>>>>>>> .max = >>>>>>>> V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, >>>>>>>> }, >>>>>>>> + }, { >>>>>>>> + .codec = HANTRO_H264_DECODER, >>>>>>>> + .cfg = { >>>>>>>> + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, >>>>>>>> + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, >>>>>>>> + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, >>>>>>>> + .menu_skip_mask = >>>>>>>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), >>>>>>>> + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, >>>>>>>> + } >>>>>>>> }, { >>>>>>>> }, >>>>>>>> }; >>
On Mon, Feb 3, 2020 at 9:00 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 2/3/20 12:13 PM, Tomasz Figa wrote: > > On Sat, Jan 18, 2020 at 10:31 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > >> > >> Le vendredi 10 janvier 2020 à 13:31 +0100, Hans Verkuil a écrit : > >>> On 11/29/19 1:16 AM, Tomasz Figa wrote: > >>>> On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne <nicolas@ndufresne.ca> > >>>> wrote: > >>>>> Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > >>>>>> On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne <nicolas@ndufresne.ca> > >>>>>> wrote: > >>>>>>> Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > >>>>>>>> The Hantro G1 decoder supports H.264 profiles from Baseline to High, > >>>>>>>> with > >>>>>>>> the exception of the Extended profile. > >>>>>>>> > >>>>>>>> Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > >>>>>>>> applications can query the driver for the list of supported > >>>>>>>> profiles. > >>>>>>> > >>>>>>> Thanks for this patch. Do you think we could also add the LEVEL > >>>>>>> control > >>>>>>> so the profile/level enumeration becomes complete ? > >>>>>>> > >>>>>>> I'm thinking it would be nice if the v4l2 compliance test make sure > >>>>>>> that codecs do implement these controls (both stateful and stateless), > >>>>>>> it's essential for stack with software fallback, or multiple capable > >>>>>>> codec hardware but with different capabilities. > >>>>>>> > >>>>>> > >>>>>> Level is a difficult story, because it also specifies the number of > >>>>>> macroblocks per second, but for decoders like this the number of > >>>>>> macroblocks per second it can handle depends on things the driver > >>>>>> might be not aware of - clock frequencies, DDR throughput, system > >>>>>> load, etc. > >>>>>> > >>>>>> My take on this is that the decoder driver should advertise the > >>>>>> highest resolution the decoder can handle due to hardware constraints. > >>>>>> Performance related things depend on the integration details and > >>>>>> should be managed elsewhere. For example Android and Chrome OS manage > >>>>>> expected decoding performance in per-board configuration files. > >>>>> > >>>>> When you read datasheet, the HW is always rated to maximum level (and > >>>>> it's a range) with the assumption of a single stream. It seems much > >>>>> easier to expose this as-is, statically then to start doing some math > >>>>> with data that isn't fully exposed to the user. This is about filtering > >>>>> of multiple CODEC instances, it does not need to be rocket science, > >>>>> specially that the amount of missing data is important (e.g. usage of > >>>>> tiles, compression, IPP all have an impact on the final performance). > >>>>> All we want to know in userspace is if this HW is even possibly capable > >>>>> of LEVEL X, and if not, we'll look for another one. > >>>>> > >>>> > >>>> Agreed, one could potentially define it this way, but would it be > >>>> really useful for the userspace and the users? I guess it could enable > >>>> slightly faster fallback to software decoding in the extreme case of > >>>> the hardware not supporting the level at all, but I suspect that the > >>>> majority of cases would be the hardware just being unusably slow. > >>>> > >>>> Also, as I mentioned before, we already return the range of supported > >>>> resolutions, which in practice should map to the part of the level > >>>> that may depend on hardware capabilities rather than performance, so > >>>> exposing levels as well would add redundancy to the information > >>>> exposed. > >>> > >>> There is a lot of discussion here, but it all revolves around a potential > >>> LEVEL control. > >>> > >>> So I gather everyone is OK with merging this patch? > >> > >> I'm ok with this. For me, the level reflects the real time performance > >> capability as define in the spec, while the width/height constraints usually > >> represent an addressing capabicity, which may or may not operate real-time. > >> > > > > I'd like to see the level control documentation improved before we > > start adding it to the drivers. I'll be okay with that then as long as > > the values are exposed in a consistent and well defined way. :) > > > > As for this patch, Hans, are you going to apply it? > > It's in a PR for 5.7. I had hoped it would go in for v5.6, but it was > too late for that. Okay, thanks! > > Regards, > > Hans > > > > > Best regards, > > Tomasz > > > >>> > >>> If not, let me know asap. > >>> > >>> Regards, > >>> > >>> Hans > >>> > >>>>>>>> Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > >>>>>>>> --- > >>>>>>>> drivers/staging/media/hantro/hantro_drv.c | 10 ++++++++++ > >>>>>>>> 1 file changed, 10 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/drivers/staging/media/hantro/hantro_drv.c > >>>>>>>> b/drivers/staging/media/hantro/hantro_drv.c > >>>>>>>> index 6d9d41170832..9387619235d8 100644 > >>>>>>>> --- a/drivers/staging/media/hantro/hantro_drv.c > >>>>>>>> +++ b/drivers/staging/media/hantro/hantro_drv.c > >>>>>>>> @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { > >>>>>>>> .def = > >>>>>>>> V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > >>>>>>>> .max = > >>>>>>>> V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, > >>>>>>>> }, > >>>>>>>> + }, { > >>>>>>>> + .codec = HANTRO_H264_DECODER, > >>>>>>>> + .cfg = { > >>>>>>>> + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > >>>>>>>> + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, > >>>>>>>> + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > >>>>>>>> + .menu_skip_mask = > >>>>>>>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > >>>>>>>> + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > >>>>>>>> + } > >>>>>>>> }, { > >>>>>>>> }, > >>>>>>>> }; > >> >
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c index 6d9d41170832..9387619235d8 100644 --- a/drivers/staging/media/hantro/hantro_drv.c +++ b/drivers/staging/media/hantro/hantro_drv.c @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = { .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B, }, + }, { + .codec = HANTRO_H264_DECODER, + .cfg = { + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, + .menu_skip_mask = + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, + } }, { }, };