'Paul Kirth (Gerrit)' via owners-override

unread,

Mar 28, 2025, 4:22:43 PM3/28/25

to Owners Override

Attention needed from Dustin Green, Martin Puryear, Owners Override and Yegor Pomortsev

Paul Kirth has uploaded the change for review

Paul Kirth would like Owners Override to review this change.

Commit message

[media][audio] Fix -Wimplicit-int-conversion errors

New versions of clang will start warning about more cases when the
conversion happens.

Bug: 402753152

Change-Id: I9d192261d03b9bfc8c5a7e63f487fb19ad6c701b

Change diff

diff --git a/src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h b/src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h
index 9ca1fd5..68300c8 100644
--- a/src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h
+++ b/src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h
@@ -376,7 +376,7 @@
       AudioBuffer<fuchsia::media::AudioSampleFormat::SIGNED_16>* audio_buffer_ptr) {
     auto& samples = audio_buffer_ptr->samples();
     for (std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT& sample : samples) {
-      sample = -sample;
+      sample = static_cast<std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT>(-sample);
     }
   }
 
@@ -499,7 +499,7 @@
       AudioBuffer<fuchsia::media::AudioSampleFormat::SIGNED_16>* audio_buffer_ptr) {
     auto& samples = audio_buffer_ptr->samples();
     for (std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT& sample : samples) {
-      sample = -sample;
+      sample = static_cast<std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT>(-sample);
     }
   }

Change information

Files:

M src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h

Change size: XS

Delta: 1 file changed, 2 insertions(+), 2 deletions(-)

Open in Gerrit

Related details

Attention is currently required from:

Dustin Green
Martin Puryear
Owners Override
Yegor Pomortsev

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/5cef959857b3bad6f5d268da6d46ea639512b5b9-HTML%40fuchsia-review.googlesource.com.

unsatisfied_requirement

open

diffy

'James Robinson (Gerrit)' via owners-override

unread,

Mar 28, 2025, 4:36:16 PM3/28/25

to Paul Kirth, Owners Override, Yegor Pomortsev, GI Try Builder, CQ Bot, Dustin Green, Martin Puryear

Attention needed from Dustin Green, Martin Puryear, Owners Override, Paul Kirth and Yegor Pomortsev

James Robinson voted and added 2 comments

Votes added by James Robinson

Code-Review	+2
Owners-Override	+1

2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

James Robinson . resolved

Setting O-O to unblock the clang roll.

I do think this is worth some follow-up by the code owners, see comment below.

File src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h

Line 379, Patchset 4 (Latest): sample = static_cast<std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT>(-sample);

James Robinson . unresolved

can we make this a local type alias to avoid repeating it in the loop and here?

this does bring up the point of what is this logic expected to do given a sample of std::numeric_limits<SampleT>::min() whose inversion is not representable in the type

Open in Gerrit

Related details

Attention is currently required from:

Dustin Green
Martin Puryear
Owners Override

Paul Kirth
Yegor Pomortsev

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/b8bc7cfc342c6397e64651d85cb0622d20bdff10-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.

satisfied_requirement

open

diffy

'Paul Kirth (Gerrit)' via owners-override

unread,

Mar 28, 2025, 5:32:19 PM3/28/25

to James Robinson, Owners Override, Yegor Pomortsev, GI Try Builder, CQ Bot, Dustin Green, Martin Puryear

Attention needed from Dustin Green, James Robinson, Martin Puryear, Owners Override and Yegor Pomortsev

Paul Kirth added 2 comments

File src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h

Line 379, Patchset 4 (Latest): sample = static_cast<std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT>(-sample);

James Robinson . resolved

can we make this a local type alias to avoid repeating it in the loop and here?
this does bring up the point of what is this logic expected to do given a sample of std::numeric_limits<SampleT>::min() whose inversion is not representable in the type

Paul Kirth

Fix applied.

Line 379, Patchset 4 (Latest): sample = static_cast<std::remove_pointer_t<decltype(audio_buffer_ptr)>::SampleT>(-sample);

James Robinson . unresolved

can we make this a local type alias to avoid repeating it in the loop and here?
this does bring up the point of what is this logic expected to do given a sample of std::numeric_limits<SampleT>::min() whose inversion is not representable in the type

Paul Kirth

Fix applied. Surprised the AI seemed to do this correctly 😊.

Not sure how to answer your question regarding the logic, though. To get around the new warning and unblock our CI I was mostly just trying to keep the existing logic in place.

I'll leave this unresolved in case you want to hold off landing.

Open in Gerrit

Related details

Attention is currently required from:

Dustin Green
James Robinson
Martin Puryear
Owners Override
Yegor Pomortsev

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/c24a3cdc2d08ad472809fed177516b4a3d4a753c-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.

satisfied_requirement

open

diffy

'Paul Kirth (Gerrit)' via owners-override

unread,

Mar 28, 2025, 5:33:08 PM3/28/25

to Auto-Submit, James Robinson, Owners Override, Yegor Pomortsev, GI Try Builder, CQ Bot, Dustin Green, Martin Puryear

Attention needed from Dustin Green, James Robinson, Martin Puryear, Owners Override and Yegor Pomortsev

Paul Kirth added 1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Paul Kirth . unresolved

Or now I'll mark it unresolved

Open in Gerrit

Related details

Attention is currently required from:

Dustin Green
James Robinson
Martin Puryear
Owners Override
Yegor Pomortsev

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--

You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/3008d80ca5c7a766a02501a773b0fad5b38968a5-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.

satisfied_requirement

open

diffy

'CQ Bot (Gerrit)' via owners-override

unread,

Mar 28, 2025, 5:41:03 PM3/28/25

to Paul Kirth, Auto-Submit, James Robinson, Owners Override, Yegor Pomortsev, GI Try Builder, Dustin Green, Martin Puryear

CQ Bot submitted the change

Change information

Commit message:

[media][audio] Fix -Wimplicit-int-conversion errors

New versions of clang will start warning about more cases when the
conversion happens.

Bug: 402753152

Change-Id: I9d192261d03b9bfc8c5a7e63f487fb19ad6c701b

Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1238872

Reviewed-by: James Robinson <jam...@google.com>

Commit-Queue: Auto-Submit <auto-...@fuchsia-infra.iam.gserviceaccount.com>

Owners-Override: James Robinson <jam...@google.com>

Fuchsia-Auto-Submit: Paul Kirth <paul...@google.com>

Files:

M src/media/audio/audio_core/test/api/audio_renderer_pipeline_test_shared.h

Change size: XS

Delta: 1 file changed, 2 insertions(+), 2 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +2 by James Robinson

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/2a38dc829d5f6267c410ae8d85a74332049347f8-HTML%40fuchsia-review.googlesource.com.

open

diffy

satisfied_requirement

'James Robinson (Gerrit)' via owners-override

unread,

Mar 28, 2025, 5:43:24 PM3/28/25

to Paul Kirth, CQ Bot, Auto-Submit, Owners Override, Yegor Pomortsev, GI Try Builder, Dustin Green, Martin Puryear

Attention needed from Paul Kirth

James Robinson added 1 comment

Patchset-level comments

File-level comment, Patchset 4:

Paul Kirth . unresolved

Or now I'll mark it unresolved

James Robinson

This is preserving behavior so I think it's valid to land and have the owners take a look later.

Open in Gerrit

Related details

Attention is currently required from:

Paul Kirth

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/8161fe05d667d8611d76751bb1a7de21f834cc06-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.

satisfied_requirement

open

diffy

'GI Roller (Gerrit)' via owners-override

unread,

Mar 28, 2025, 5:44:09 PM3/28/25

to Paul Kirth, CQ Bot, Auto-Submit, James Robinson, Owners Override, Yegor Pomortsev, GI Try Builder, Dustin Green, Martin Puryear

Attention needed from Paul Kirth

Message from GI Roller

Change has been successfully rolled: http://go/roll-cl/5e2c4dbec26ad1971011becb006549a04ca9d420

Open in Gerrit

Related details

Attention is currently required from:

Paul Kirth

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/1cd5b42b5452911298604c684c951b1f5eea85fc-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.

satisfied_requirement

open

diffy

'Paul Kirth (Gerrit)' via owners-override

unread,

Mar 28, 2025, 5:45:48 PM3/28/25

to CQ Bot, GI Roller, Auto-Submit, James Robinson, Owners Override, Yegor Pomortsev, GI Try Builder, Dustin Green, Martin Puryear

Paul Kirth added 1 comment

Patchset-level comments

File-level comment, Patchset 4:

Paul Kirth . resolved

Or now I'll mark it unresolved

James Robinson

This is preserving behavior so I think it's valid to land and have the owners take a look later.

Paul Kirth

Acknowledged

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.

To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/58976db91884b75081cb26f38e08ddb3f55d14d5-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[XS] Change in fuchsia/fuchsia[main]: [media][audio] Fix -Wimplicit-int-conversion errors

'Paul Kirth (Gerrit)' via owners-override

Paul Kirth has uploaded the change for review

Commit message

Change diff

Change information

Related details

'James Robinson (Gerrit)' via owners-override

James Robinson voted and added 2 comments

Votes added by James Robinson

2 comments

Related details

'Paul Kirth (Gerrit)' via owners-override

Paul Kirth added 2 comments

Related details

'Paul Kirth (Gerrit)' via owners-override

Paul Kirth added 1 comment

Related details

'CQ Bot (Gerrit)' via owners-override

CQ Bot submitted the change

Change information

'James Robinson (Gerrit)' via owners-override

James Robinson added 1 comment

Related details

'GI Roller (Gerrit)' via owners-override

Message from GI Roller

Related details

'Paul Kirth (Gerrit)' via owners-override

Paul Kirth added 1 comment

Related details