On 1/9/20 4:17 PM, Jernej Škrabec wrote:
> Hi!
>
> Dne sreda, 08. januar 2020 ob 15:43:36 CET je Paul Kocialkowski napisal(a):
>> Hi Mauro,
>>
>> On Wed 08 Jan 20, 15:11, Mauro Carvalho Chehab wrote:
>>> Em Fri, 13 Dec 2019 17:04:25 +0100
>>>
>>> Jernej Skrabec <jernej.skrabec@xxxxxxxx> escreveu:
>>>> HEVC has a scaling matrix concept. Add support for it.
>>>>
>>>> +struct v4l2_ctrl_hevc_scaling_matrix {
>>>> + __u8 scaling_list_4x4[6][16];
>>>> + __u8 scaling_list_8x8[6][64];
>>>> + __u8 scaling_list_16x16[6][64];
>>>> + __u8 scaling_list_32x32[2][64];
>>>> + __u8 scaling_list_dc_coef_16x16[6];
>>>> + __u8 scaling_list_dc_coef_32x32[2];
>>>> +};
>>>
>>> I never looked at HEVC spec, but the above seems really weird.
>>>
>>> Please correct me if I am wrong, but each of the above matrixes
>>> is independent, and the driver will use just one of the above on
>>> any specific time (for a given video output node), right?
>>
>> I am not too sure about what the specification really entails, but it is my
>> understanding that HEVC allows simultaneous block sizes between 4x4 and
>> 32x32 to exist within the same coding tree and slice. That suggests that it
>> makes sense to have specific coefficients for each case.
>
> Specs ITU-T REC. H.265 (06/2019), chapter 7.3.4 shows that multiple different
> matrices can be present at the same time. If they are not, default values
> should be used instead. But in general, more than one can be needed at the
> same time.
>
> Only real question is if default values should be also provided by userspace
> or by kernel. Since place has to be reserved for all different scaling lists
> anyway, we won't save any space by providing default values in kernel. Cedrus
> VPU has only bit switch for using default values for all matrices at the same
> time or all custom.
>
> Note that this control contains slightly processed data. Frame has stored
> these matrices in form of deltas. But because this is the only driver that use
> this structure I have no idea what is the most proper form of this data (raw
> values or deltas). That's why this will stay in staging using private headers
> until we figure this out.
This definitely needs to be documented! Otherwise this will be forgotten.
Regards,
Hans
>
> Best regards,
> Jernej
>
>>
>> Note that the hardware also has distinct registers for each scaling list.
>>
>> Cheers,
>>
>> Paul
>>
>>> If so, why would userspace be forced to update lots of matrixes, if would
>>> likely use just one at a given time?
>>>
>>> IMO, the proper way would be, instead, to use an uAPI like:
>>>
>>> /*
>>>
>>> * Actually, as this is uAPI, we will use a fixed size integer type, like
>>> * unsigned int
>>> */
>>>
>>> enum hevc_scaling_matrix_type {
>>>
>>> HEVC_SCALING_MATRIX_4x4,
>>> HEVC_SCALING_MATRIX_8x8,
>>>
>>> ...
>>>
>>> HEVC_SCALING_MATRIX_DC_COEF_32x32,
>>>
>>> };
>>>
>>> struct v4l2_ctrl_hevc_scaling_matrix {
>>>
>>> __u32 scaling_type /* as defined by enum
> hevc_scaling_matrix_type */
>>>
>>> union {
>>>
>>> __u8 scaling_list_4x4[6][16];
>>> __u8 scaling_list_8x8[6][64];
>>> __u8 scaling_list_16x16[6][64];
>>> __u8 scaling_list_32x32[2][64];
>>> __u8 scaling_list_dc_coef_16x16[6];
>>> __u8 scaling_list_dc_coef_32x32[2];
>>>
>>> };
>>>
>>> };
>>>
>>> And let the core use a default for each scaling matrix, if userspace
>>> doesn't set it.
>>>
>>>
>>>
>>> Cheers,
>>> Mauro
>
>
>
>
