[PATCH v3 3/9] video: Add generic HDMI infoframe helpers

[Thierry Reding]

On Wed, Jan 16, 2013 at 03:27:14PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 14, 2013 at 03:30:22PM +0100, Thierry Reding wrote:
> > Add generic helpers to pack HDMI infoframes into binary buffers.
[...]
> > +/**
> > + * hdmi_avi_infoframe_init() - initialize an HDMI AVI infoframe
> > + * @frame: HDMI AVI infoframe
> > + *
> > + * Returns 0 on success or a negative error code on failure.
> > + */
> > +int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame)
> > +{
> > +	if (!frame)
> > +		return -EINVAL;
> 
> There's quite a bit of error checking all around which seems a bit
> pointless since this is all internal stuff. Any errors are bugs in
> some driver code, so just letting the thing oops would be fine in my
> opinion.

Okay, I can remove some of the more obvious checks. Checking for space
requirements might still be useful, so I'll leave that in.

> > +ssize_t hdmi_spd_infoframe_pack(struct hdmi_spd_infoframe *frame, void *buffer,
> > +				size_t size)
[...]
> > +	for (i = 0; i < sizeof(frame->vendor); i++)
> > +		ptr[i] = frame->vendor[i];
> > +
> > +	for (i = 0; i < sizeof(frame->product); i++)
> > +		ptr[8 + i] = frame->product[i];
> 
> memcpy()

Done.

> > +enum hdmi_spd_sdi {
> > +	HDMI_SPD_SDI_UNKNOWN,
> > +	HDMI_SPD_SDI_DSTB,
> > +	HDMI_SPD_SDI_DVDP,
> > +	HDMI_SPD_SDI_DVHS,
> > +	HDMI_SPD_SDI_HDDVR,
> > +	HDMI_SPD_SDI_DVC,
> > +	HDMI_SPD_SDI_DSC,
> > +	HDMI_SPD_SDI_VCD,
> > +	HDMI_SPD_SDI_GAME,
> > +	HDMI_SPD_SDI_PC,
> > +	HDMI_SPD_SDI_BD,
> > +	HDMI_SPD_SDI_SCD,
> 
> I believe SACD is a more correct name.

Done.

> HD DVD and PMP are missing from this list.

I can't find any reference to those in CEA-861-D. HDMI 1.4 doesn't have
them either. Can you provide any pointers?

> > +enum hdmi_audio_coding_type {
> > +	HDMI_AUDIO_CODING_TYPE_STREAM,
> > +	HDMI_AUDIO_CODING_TYPE_IEC_60958,
> > +	HDMI_AUDIO_CODING_TYPE_AC3,
> > +	HDMI_AUDIO_CODING_TYPE_MPEG1,
> > +	HDMI_AUDIO_CODING_TYPE_MP3,
> > +	HDMI_AUDIO_CODING_TYPE_MPEG2,
> > +	HDMI_AUDIO_CODING_TYPE_AAC,
> > +	HDMI_AUDIO_CODING_TYPE_DTS,
> > +	HDMI_AUDIO_CODING_TYPE_ATRAC,
> > +	HDMI_AUDIO_CODING_TYPE_ONE_BIT_AUDIO,
> > +	HDMI_AUDIO_CODING_TYPE_DOLBY_DIGITAL_PLUS,
> > +	HDMI_AUDIO_CODING_TYPE_DTS_HD,
> > +	HDMI_AUDIO_CODING_TYPE_MAT_MLP,
> > +	HDMI_AUDIO_CODING_TYPE_DST,
> > +	HDMI_AUDIO_CODING_TYPE_WMPRO,
> 
> These don't always quite match the names in CEA-861-E. Wouldn't it
> be better to use matching names?

Are HD-DVD and PMP above listed in CEA-861-E? I'll need to find a copy
of it and will go over this again and match the names to those in the
document.

> Also the audio coding extension type is missing.

I assume this is also part of CEA-861-E as I can't find a reference in
-D?

Thanks for reviewing,
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130118/8d8550e5/attachment.pgp>