[igt-dev] [PATCH i-g-t 3/5] lib/igt_eld: parse Short Audio Descriptors
Ser, Simon
simon.ser at intel.com
Mon Jun 3 15:06:25 UTC 2019
On Mon, 2019-06-03 at 15:57 +0300, Martin Peres wrote:
> On 31/05/2019 14:21, Simon Ser wrote:
> > Each valid ELD entry can contain zero, one or more Short Audio Descriptor
> > blocks. These are exposed in sadN_* fields (N being the index of the SAD).
> >
> > We need to parse them to be able to check that ALSA has properly processed
> > them.
> >
> > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > ---
> > lib/igt_edid.h | 2 ++
> > lib/igt_eld.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > lib/igt_eld.h | 14 +++++++++
> > 3 files changed, 100 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> > index 7edd7e38f41e..1d0c6aa29578 100644
> > --- a/lib/igt_edid.h
> > +++ b/lib/igt_edid.h
> > @@ -30,6 +30,8 @@
> >
> > #include <stdint.h>
> >
> > +#include <xf86drmMode.h>
> > +
>
> Are you sure this change belongs to this commit? Can't find anything
> here that would need this header.
Fair, I've split it into another commit
> > struct est_timings {
> > uint8_t t1;
> > uint8_t t2;
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index a198001a56d7..732bbabd2d7c 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -35,6 +35,7 @@
> >
> > #define ELD_PREFIX "eld#"
> > #define ELD_DELIM " \t"
> > +#define SAD_FMT "sad%d_%ms"
> >
> > /**
> > * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and
> > @@ -43,13 +44,87 @@
> > * on).
> > */
> >
> > +static enum cea_sad_format parse_sad_coding_type(const char *value)
> > +{
> > + if (strcmp(value, "LPCM") == 0)
> > + return CEA_SAD_FORMAT_PCM;
> > + else
> > + return 0;
> > +}
> > +
> > +static enum cea_sad_sampling_rate parse_sad_rate(const char *value)
> > +{
> > + switch (atoi(value)) {
> > + case 32000:
> > + return CEA_SAD_SAMPLING_RATE_32KHZ;
> > + case 44100:
> > + return CEA_SAD_SAMPLING_RATE_44KHZ;
> > + case 48000:
> > + return CEA_SAD_SAMPLING_RATE_48KHZ;
> > + case 88000:
> > + return CEA_SAD_SAMPLING_RATE_88KHZ;
> > + case 96000:
> > + return CEA_SAD_SAMPLING_RATE_96KHZ;
> > + case 176000:
> > + return CEA_SAD_SAMPLING_RATE_176KHZ;
> > + case 192000:
> > + return CEA_SAD_SAMPLING_RATE_192KHZ;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static enum cea_sad_pcm_sample_size parse_sad_bit(const char *value)
> > +{
> > + switch (atoi(value)) {
> > + case 16:
> > + return CEA_SAD_SAMPLE_SIZE_16;
> > + case 20:
> > + return CEA_SAD_SAMPLE_SIZE_20;
> > + case 24:
> > + return CEA_SAD_SAMPLE_SIZE_24;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static void parse_sad_field(struct eld_sad *sad, const char *key, char *value)
> > +{
> > + char *tok;
> > +
> > + /* Some fields are prefixed with the raw hex value, strip it */
> > + if (value[0] == '[') {
> > + value = strchr(value, ' ');
> > + igt_assert(value != NULL);
> > + value++; /* skip the space */
> > + }
> > +
> > + /* Single-value fields */
> > + if (strcmp(key, "coding_type") == 0)
> > + sad->coding_type = parse_sad_coding_type(value);
> > + else if (strcmp(key, "channels") == 0)
> > + sad->channels = atoi(value);
> > +
> > + /* Multiple-value fields */
> > + tok = strtok(value, " ");
> > + while (tok) {
> > + if (strcmp(key, "rates") == 0)
> > + sad->rates |= parse_sad_rate(tok);
> > + else if (strcmp(key, "bits") == 0)
> > + sad->bits |= parse_sad_bit(tok);
> > +
> > + tok = strtok(NULL, " ");
> > + }
> > +}
> > +
> > static bool eld_parse_entry(const char *path, struct eld_entry *eld)
> > {
> > FILE *f;
> > char buf[1024];
> > - char *key, *value;
> > + char *key, *value, *sad_key;
>
> Poor key, why are you sad? :D
So many sad symbols! It's a shame EDID doesn't include any HAPPY
fields…
> More seriously, a little documentation in this file about the acronyms
> would be appreciated!
Agreed. I'll add a note about this. And I'll send a patch for igt_edid
docs.
> > size_t len;
> > bool monitor_present;
> > + int sad_index;
> >
> > memset(eld, 0, sizeof(*eld));
> >
> > @@ -76,6 +151,14 @@ static bool eld_parse_entry(const char *path, struct eld_entry *eld)
> > else if (strcmp(key, "monitor_name") == 0)
> > snprintf(eld->monitor_name, sizeof(eld->monitor_name),
> > "%s", value);
> > + else if (strcmp(key, "sad_count") == 0)
> > + eld->sads_len = atoi(value);
> > + else if (sscanf(key, "sad%d_%ms", &sad_index, &sad_key) == 2) {
> > + igt_assert(sad_index < ELD_SADS_CAP);
> > + igt_assert(sad_index < eld->sads_len);
>
> Good that you added that! This patch looks good:
>
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
>
> > + parse_sad_field(&eld->sads[sad_index], sad_key, value);
> > + free(sad_key);
> > + }
> > }
> >
> > if (ferror(f) != 0) {
> > diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> > index 21fe3537acf9..e16187884d4b 100644
> > --- a/lib/igt_eld.h
> > +++ b/lib/igt_eld.h
> > @@ -30,9 +30,23 @@
> >
> > #include <stdbool.h>
> >
> > +#include "igt_edid.h"
> > +
> > +#define ELD_SADS_CAP 4
> > +
> > +/** eld_sad: Short Audio Descriptor */
> > +struct eld_sad {
> > + enum cea_sad_format coding_type;
> > + int channels;
> > + unsigned int rates; /* enum cea_sad_sampling_rate */
> > + unsigned int bits; /* enum cea_sad_pcm_sample_size */
> > +};
> > +
> > struct eld_entry {
> > bool valid;
> > char monitor_name[16];
> > + size_t sads_len;
> > + struct eld_sad sads[ELD_SADS_CAP];
> > };
> >
> > bool eld_has_igt(void);
> >
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list