Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature multiple loudness metadata squash tests #110

Merged
merged 9 commits into from
Jun 29, 2021

Conversation

rsjbailey
Copy link
Contributor

@rsjbailey rsjbailey commented Jun 2, 2021

fixes #84

Use generic parameter tests
Add a generic test for vector parameters
Allow a custom comparator for vector parameters (defaults to using operator==)
Fix missing unset(ContentKind) implementation

@rsjbailey rsjbailey force-pushed the feature_multiple_loudnessMetadata_squash_tests branch 2 times, most recently from 2b68021 to f54d6cb Compare June 2, 2021 19:36
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2021

Codecov Report

Merging #110 (4e13581) into 2076-2 (bba67f8) will decrease coverage by 0.43%.
The diff coverage is 82.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##           2076-2     #110      +/-   ##
==========================================
- Coverage   90.78%   90.35%   -0.44%     
==========================================
  Files         122      120       -2     
  Lines        5700     5557     -143     
==========================================
- Hits         5175     5021     -154     
- Misses        525      536      +11     
Impacted Files Coverage Δ
...clude/adm/elements/audio_block_format_binaural.hpp 93.75% <ø> (-0.37%) ⬇️
...dm/elements/audio_block_format_direct_speakers.hpp 100.00% <ø> (ø)
include/adm/elements/audio_block_format_hoa.hpp 100.00% <ø> (ø)
include/adm/elements/audio_block_format_id.hpp 100.00% <ø> (ø)
include/adm/elements/audio_block_format_matrix.hpp 93.75% <ø> (-0.37%) ⬇️
...nclude/adm/elements/audio_block_format_objects.hpp 100.00% <ø> (ø)
include/adm/elements/audio_channel_format.hpp 94.64% <ø> (-0.10%) ⬇️
include/adm/elements/audio_channel_format_id.hpp 100.00% <ø> (ø)
include/adm/elements/audio_content_id.hpp 100.00% <ø> (ø)
include/adm/elements/audio_object.hpp 100.00% <ø> (ø)
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3dc6838...4e13581. Read the comment docs.

@tomjnixon
Copy link
Member

If add finds a duplicate, should it replace it rather than just returning false? If the comparator checks the full value (like operator==) then the current behaviour makes sense, but if it just checks a key (like a language for example) then the new value should replace the old one.

@rsjbailey
Copy link
Contributor Author

If add finds a duplicate, should it replace it rather than just returning false? If the comparator checks the full value (like operator==) then the current behaviour makes sense, but if it just checks a key (like a language for example) then the new value should replace the old one.

That makes sense, but add() then becomes a bit of a misleading name. I guess the stl equivalent would be insert_or_assign, although it's not especially catchy.

@tomjnixon
Copy link
Member

Yeah, good point. Another alternative would be to use set/unset for things like this which are expected to be unique.

As long as it's documented it's probably OK.

@rsjbailey rsjbailey force-pushed the feature_multiple_loudnessMetadata_squash_tests branch from 4a9d2a7 to 7a44f62 Compare June 16, 2021 15:14
@rsjbailey rsjbailey force-pushed the feature_multiple_loudnessMetadata_squash_tests branch from 6163f28 to 4e13581 Compare June 29, 2021 09:51
@rsjbailey rsjbailey force-pushed the feature_multiple_loudnessMetadata_squash_tests branch from 4e13581 to daeef33 Compare June 29, 2021 10:31
@rsjbailey rsjbailey marked this pull request as ready for review June 29, 2021 10:33
@rsjbailey rsjbailey merged commit 2049db3 into 2076-2 Jun 29, 2021
@tomjnixon tomjnixon deleted the feature_multiple_loudnessMetadata_squash_tests branch November 19, 2021 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow multiple loudnessMetadata elements in audioProgramme and audioContent
4 participants