lib/igt_audio: normalize samples when extracting channel

PATCH In review Simon Ser 3 participants 6 messages May 13, 2019 – May 14, 2019 Library code

Share & tools

Simon Ser

author

2019-05-13 14:11 UTC #0 lore raw

PATCH 1/1 lib/igt_audio: normalize samples when extracting channel

audio_extract_channel_s32_le converts S32 samples to double. Prior to this patch, the output samples weren't normalized, resulting in values up to INT32_MAX.

Future amplitude tests need to be format-agnostic (ie. shouldn't care whether the format used is S16 or S32). Thus, it is preferable to always use normalized double values.

The current tests don't check the amplitude (or check it relative to the max. on the whole buffer), so they still pass without any modification.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/igt_audio.c +1 -1

index fd8cf07c0de3..f4b169595f6c 100644
@@ -389,7 +389,7 @@ size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
 	dst_len = src_len / n_channels;
 	igt_assert(dst_len <= dst_cap);
 	for (i = 0; i < dst_len; i++)
-		dst[i] = (double) src[i * n_channels + channel];
+		dst[i] = (double) src[i * n_channels + channel] / INT32_MAX;
 
 	return dst_len;
 }

--
2.21.0

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Patchwork

2019-05-13 15:38 UTC #1 lore raw

[igt-dev] ✓ Fi.CI.BAT: success for lib/igt_audio: normalize samples when extracting channel

== Series Details ==

Series: lib/igt_audio: normalize samples when extracting channel
URL : https://patchwork.freedesktop.org/series/60586/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6076 -> IGTPW_2973
====================================================

Summary
-------

**SUCCESS**

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/60586/revisions/1/mbox/

Known issues
------------

Here are the changes found in IGTPW_2973 that come from known issues:

### IGT changes ###

#### Possible fixes ####

* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [INCOMPLETE][1] ([fdo#108602] / [fdo#108744]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html

* igt@kms_chamelium@dp-crc-fast:
- {fi-cml-u2}: [FAIL][3] ([fdo#110627]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/fi-cml-u2/igt@[email protected]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/fi-cml-u2/igt@[email protected]

#### Warnings ####

* igt@i915_selftest@live_hangcheck:
- fi-apl-guc: [DMESG-FAIL][5] ([fdo#110620]) -> [FAIL][6] ([fdo#110623])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/fi-apl-guc/igt@i915_selftest@live_hangcheck.html

{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).

[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#110620]: https://bugs.freedesktop.org/show_bug.cgi?id=110620
[fdo#110623]: https://bugs.freedesktop.org/show_bug.cgi?id=110623
[fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627

Participating hosts (52 -> 45)
------------------------------

Additional (1): fi-pnv-d510
Missing (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus

Build changes
-------------

* IGT: IGT_4983 -> IGTPW_2973

CI_DRM_6076: 71dc43d6470737153d1859ea941436fc39f8fbc2 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2973: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/
IGT_4983: 7455cff58bfaba946b70e62e354851feb5f55f3a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Ville Syrjälä

2019-05-13 16:25 UTC #2 lore raw

In reply to Simon Ser

Quoted 21 lines · Simon Ser

audio_extract_channel_s32_le converts S32 samples to double. Prior to this patch, the output samples weren't normalized, resulting in values up to INT32_MAX.
Future amplitude tests need to be format-agnostic (ie. shouldn't care whether the format used is S16 or S32). Thus, it is preferable to always use normalized double values.
The current tests don't check the amplitude (or check it relative to the max. on the whole buffer), so they still pass without any modification.
Signed-off-by Simon Ser <[email protected]>
---
 lib/igt_audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/igt_audio.c +0 -0
index fd8cf07c0de3..f4b169595f6c 100644

lib/igt_audio.c size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap, · line 392

Quoted 5 lines

lib/igt_audio.c +1 -1

@@ -389,7 +389,7 @@ size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
 	dst_len = src_len / n_channels;
 	igt_assert(dst_len <= dst_cap);
 	for (i = 0; i < dst_len; i++)
-		dst[i] = (double) src[i * n_channels + channel];
+		dst[i] = (double) src[i * n_channels + channel] / INT32_MAX;

Do we care that the most negative value could result in a normalized value < -1.0?

Quoted 10 lines

lib/igt_audio.c +0 -1
 	return dst_len;
 }
--
2.21.0
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Signature · 6 lines

Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Ser, Simon

author

2019-05-14 06:33 UTC #3 lore raw

Quoted 5 lines · Ville Syrjälä

lib/igt_audio.c size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap, · line 392
Quoted 2 lines
lib/igt_audio.c +1 -1
-		dst[i] = (double) src[i * n_channels + channel];
+		dst[i] = (double) src[i * n_channels + channel] / INT32_MAX;
Do we care that the most negative value could result in a normalized value < -1.0?

Ah, good point.

I'd be tempted not to care. These values are just used to compare the amplitude of the signal we send to the amplitude of the signal we receive.

However I agree this could be unexpected, especially if someone uses these values for another goal. If we wanted to fix it, would -(double)INT32_MIN be enough? (double can represent exact integers up to 2^53 - 1)
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Ville Syrjälä

Reviewed-by

2019-05-14 12:29 UTC #4 lore raw

Quoted 9 lines · Simon

Quoted 5 lines · Ville Syrjälä
lib/igt_audio.c size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap, · line 392
Quoted 2 lines
lib/igt_audio.c +1 -1
-		dst[i] = (double) src[i * n_channels + channel];
+		dst[i] = (double) src[i * n_channels + channel] / INT32_MAX;
Do we care that the most negative value could result in a normalized value < -1.0?
Ah, good point.

Quoted 8 lines

I'd be tempted not to care. These values are just used to compare the amplitude of the signal we send to the amplitude of the signal we receive.
However I agree this could be unexpected, especially if someone uses these values for another goal. If we wanted to fix it, would -(double)INT32_MIN be enough? (double can represent exact integers up to 2^53 - 1)

Whatever works for you.

Either way

Reviewed-by Ville Syrjälä <[email protected]>

Signature · 6 lines

Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Patchwork

2019-05-13 18:36 UTC #5 lore raw

[igt-dev] ✓ Fi.CI.IGT: success for lib/igt_audio: normalize samples when extracting channel

In reply to Simon Ser

== Series Details ==

Series: lib/igt_audio: normalize samples when extracting channel
URL : https://patchwork.freedesktop.org/series/60586/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6076_full -> IGTPW_2973_full
====================================================

Summary
-------

**SUCCESS**

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/60586/revisions/1/mbox/

Known issues
------------

Here are the changes found in IGTPW_2973_full that come from known issues:

### IGT changes ###

#### Issues hit ####

* igt@i915_pm_rc6_residency@rc6-accuracy:
- shard-snb: [PASS][1] -> [SKIP][2] ([fdo#109271])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-snb4/igt@[email protected]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-snb1/igt@[email protected]

* igt@kms_cursor_crc@cursor-64x64-onscreen:
- shard-kbl: [PASS][3] -> [FAIL][4] ([fdo#103232])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl2/igt@[email protected]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl6/igt@[email protected]
- shard-apl: [PASS][5] -> [FAIL][6] ([fdo#103232])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-apl5/igt@[email protected]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-apl2/igt@[email protected]

* igt@kms_cursor_crc@cursor-alpha-opaque:
- shard-kbl: [PASS][7] -> [FAIL][8] ([fdo#109350])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl3/igt@[email protected]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl7/igt@[email protected]
- shard-apl: [PASS][9] -> [FAIL][10] ([fdo#109350])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-apl6/igt@[email protected]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-apl1/igt@[email protected]

* igt@kms_cursor_legacy@cursor-vs-flip-toggle:
- shard-hsw: [PASS][11] -> [FAIL][12] ([fdo#103355])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-hsw1/igt@[email protected]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-hsw6/igt@[email protected]

* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
- shard-iclb: [PASS][13] -> [FAIL][14] ([fdo#103167]) +1 similar issue
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html

* igt@kms_psr@psr2_cursor_plane_onoff:
- shard-iclb: [PASS][15] -> [SKIP][16] ([fdo#109441]) +2 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html

* igt@kms_sysfs_edid_timing:
- shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#100047])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb5/igt@kms_sysfs_edid_timing.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb3/igt@kms_sysfs_edid_timing.html

* igt@kms_vblank@pipe-c-ts-continuation-suspend:
- shard-apl: [PASS][19] -> [DMESG-WARN][20] ([fdo#108566])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-apl8/igt@[email protected]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-apl7/igt@[email protected]

#### Possible fixes ####

* igt@kms_cursor_crc@cursor-64x21-onscreen:
- shard-kbl: [FAIL][21] ([fdo#103232]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl1/igt@[email protected]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl1/igt@[email protected]

* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
- shard-glk: [FAIL][23] ([fdo#104873]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-glk8/igt@[email protected]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-glk5/igt@[email protected]

* igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
- shard-glk: [FAIL][25] ([fdo#103060]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-glk6/igt@[email protected]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-glk5/igt@[email protected]

* igt@kms_flip@flip-vs-fences:
- shard-kbl: [DMESG-WARN][27] ([fdo#103558] / [fdo#105602]) -> [PASS][28] +18 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl7/igt@[email protected]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl2/igt@[email protected]

* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
- shard-iclb: [FAIL][29] ([fdo#103167]) -> [PASS][30] +6 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- shard-apl: [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +4 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-apl3/igt@[email protected]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-apl5/igt@[email protected]

* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [FAIL][33] ([fdo#103166]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb1/igt@[email protected]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb7/igt@[email protected]

* igt@kms_psr@psr2_basic:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36] +3 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb4/igt@kms_psr@psr2_basic.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb2/igt@kms_psr@psr2_basic.html

* igt@perf_pmu@rc6:
- shard-kbl: [SKIP][37] ([fdo#109271]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl2/igt@[email protected]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl1/igt@[email protected]

* igt@perf_pmu@rc6-runtime-pm-long:
- shard-iclb: [FAIL][39] ([fdo#105010]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-iclb8/igt@[email protected]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-iclb6/igt@[email protected]

#### Warnings ####

* igt@kms_busy@basic-flip-f:
- shard-apl: [INCOMPLETE][41] ([fdo#103927]) -> [SKIP][42] ([fdo#109271] / [fdo#109278])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-apl4/igt@[email protected]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-apl6/igt@[email protected]

* igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-shrfb-draw-mmap-gtt:
- shard-kbl: [SKIP][43] ([fdo#105602] / [fdo#109271]) -> [SKIP][44] ([fdo#109271]) +8 similar issues
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl7/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-shrfb-draw-mmap-gtt.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl7/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-shrfb-draw-mmap-gtt.html

* igt@kms_pipe_crc_basic@read-crc-pipe-d:
- shard-kbl: [SKIP][45] ([fdo#105602] / [fdo#109271] / [fdo#109278]) -> [SKIP][46] ([fdo#109271] / [fdo#109278])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6076/shard-kbl4/igt@[email protected]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2973/shard-kbl6/igt@[email protected]

Participating hosts (10 -> 6)
------------------------------

Missing (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005

Build changes
-------------

* IGT: IGT_4983 -> IGTPW_2973
* Piglit: piglit_4509 -> None

== Logs ==