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
Attachment:
pgp_WzveNEvby.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
