[VDPAU] [PATCH] Add tracing for HEVC picture info

[Jose Soltren]

This looks okay to me. We can address the changes to the convention for
printf() and variable type casting in a follow on change. I do agree that
Rémi's proposal is a good one.

Reviewed-by: José Hiram Soltren <jsoltren at nvidia.com>

--
nvpublic

On 2015/05/12, 7:36 , "Bibhuti Prusty" <bprusty at nvidia.com> wrote:

>Thanks for the review. Response in-lined below.
>
>> -----Original Message-----
>> From: Rémi Denis-Courmont [mailto:remi at remlab.net]
>> Sent: Monday, May 11, 2015 11:21 PM
>> To: vdpau at lists.freedesktop.org; Bibhuti Prusty
>> Subject: Re: [VDPAU] [PATCH] Add tracing for HEVC picture info
>> 
>> 	Hello,
>> 
>> Le lundi 11 mai 2015, 17:26:17 Bibhuti Prusty a écrit :
>> > +    case VDP_DECODER_PROFILE_HEVC_MAIN:
>> > +    case VDP_DECODER_PROFILE_HEVC_MAIN_10:
>> > +    case VDP_DECODER_PROFILE_HEVC_MAIN_STILL:
>> > +    case VDP_DECODER_PROFILE_HEVC_MAIN_12:
>> > +    case VDP_DECODER_PROFILE_HEVC_MAIN_444:
>> > +        {
>> > +            VdpPictureInfoHEVC const * picture_info_hevc =
>> > +                (VdpPictureInfoHEVC const *)picture_info;
>> > +
>> > +            fprintf(
>> > +                _vdp_cap_data.fp,
>> > +                "{%u, %u, %u, %u, %u, %u, %u, %u, %u, %u, %u, %u, %u,
>> %u,
>> > %u, {",
>> 
>> Strictly speaking, the format strings for uint32_t and int32_t are
>>"%"PRIu32
>> and %"PRId32 respectively.
>
>I used the existing conventions in the code.
>
>> Though to get them in C++ code, you need;
>> #define __STDC_FORMAT_MACROS 1
>> 
>> > +                (uint32_t)picture_info_hevc->chroma_format_idc,
>> > +                (uint32_t)picture_info_hevc-
>> >separate_colour_plane_flag,
>> > +                (uint32_t)picture_info_hevc-
>> >pic_width_in_luma_samples,
>> > +                (uint32_t)picture_info_hevc-
>> >pic_height_in_luma_samples,
>> > +                (uint32_t)picture_info_hevc->bit_depth_luma_minus8,
>> > +                (uint32_t)picture_info_hevc->bit_depth_chroma_minus8,
>> 
>> Why all the casts? Why not use the correct format strings?
>
>I don¹t know the history behind it, but again, I followed the existing
>syntax of dumping the variables as 32-bit values.
>
>Thanks
>./Bibhuti
>[nvpublic]
>
>> > +
>> > (uint32_t)picture_info_hevc->log2_max_pic_order_cnt_lsb_minus4, +
>> >      (uint32_t)picture_info_hevc->sps_max_dec_pic_buffering_minus1, +
>> >
>> > (uint32_t)picture_info_hevc->log2_min_luma_coding_block_size_minus3, +
>> >
>> > (uint32_t)picture_info_hevc->log2_diff_max_min_luma_coding_block_size,
>> +
>> >
>> > (uint32_t)picture_info_hevc->log2_min_transform_block_size_minus2, +
>> >
>> > (uint32_t)picture_info_hevc->log2_diff_max_min_transform_block_size, +
>> >           (uint32_t)picture_info_hevc-
>> >max_transform_hierarchy_depth_inter,
>> > +
>> > (uint32_t)picture_info_hevc->max_transform_hierarchy_depth_intra, +
>> >        (uint32_t)picture_info_hevc->scaling_list_enabled_flag+
>> > );
>> 
>> --
>> Rémi Denis-Courmont
>> NVIDIA Helsinki Oy
>> http://www.remlab.net/
>
>_______________________________________________
>VDPAU mailing list
>VDPAU at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/vdpau