[NdkVEA] Implement native temporal layer ID retrieval. [chromium/src : main]
Eugene Zemtsov (Gerrit)
Eugene Zemtsov added 4 comments![]( "Open in Gerrit")
TemporalScalabilityIdExtractor::BitstreamMetadata bits_md;
if (!svc_parser_->ParseChunk(out_buffer_data, input_since_keyframe_count_,
bits_md)) {
NotifyErrorStatus({EncoderStatus::Codes::kEncoderHardwareDriverError,
"Parse bitstream failed"});
return;
}
temporal_id = bits_md.temporal_id;let's do this only for H264
static constexpr char AMEDIAFORMAT_KEY_TEMPORAL_LAYER_ID[] =string_view?
if (!base::FeatureList::IsEnabled(move this check behind the version check
return base::android::android_info::sdk_int() >= 37;prefer `__builtin_available(android 37, *)`
Related details
- Phil Yan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan added 1 comment![]( "Open in Gerrit")
TemporalScalabilityIdExtractor::BitstreamMetadata bits_md;
if (!svc_parser_->ParseChunk(out_buffer_data, input_since_keyframe_count_,
bits_md)) {
NotifyErrorStatus({EncoderStatus::Codes::kEncoderHardwareDriverError,
"Parse bitstream failed"});
return;
}
temporal_id = bits_md.temporal_id;let's do this only for H264
please correct me if I misunderstood - if we also execute this fallback path, which is the existing temporal id retrieval logic, for H264, won't AV1 and VP9 be affected before the native SVC layer id support is enabled? Or do you mean we only assign the temporal id for H264 in the switch statement below?
Related details
- Eugene Zemtsov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan added 1 comment![]( "Open in Gerrit")
TemporalScalabilityIdExtractor::BitstreamMetadata bits_md;
if (!svc_parser_->ParseChunk(out_buffer_data, input_since_keyframe_count_,
bits_md)) {
NotifyErrorStatus({EncoderStatus::Codes::kEncoderHardwareDriverError,
"Parse bitstream failed"});
return;
}
temporal_id = bits_md.temporal_id;Phil Yanlet's do this only for H264
please correct me if I misunderstood - if we also execute this fallback path, which is the existing temporal id retrieval logic, for H264, won't AV1 and VP9 be affected before the native SVC layer id support is enabled? Or do you mean we only assign the temporal id for H264 in the switch statement below?
typo: if we *only* execute this fallback path...
Related details
- Eugene Zemtsov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan added 1 comment![]( "Open in Gerrit")
TemporalScalabilityIdExtractor::BitstreamMetadata bits_md;
if (!svc_parser_->ParseChunk(out_buffer_data, input_since_keyframe_count_,
bits_md)) {
NotifyErrorStatus({EncoderStatus::Codes::kEncoderHardwareDriverError,
"Parse bitstream failed"});
return;
}
temporal_id = bits_md.temporal_id;Phil Yanlet's do this only for H264
Phil Yanplease correct me if I misunderstood - if we also execute this fallback path, which is the existing temporal id retrieval logic, for H264, won't AV1 and VP9 be affected before the native SVC layer id support is enabled? Or do you mean we only assign the temporal id for H264 in the switch statement below?
typo: if we *only* execute this fallback path...
oh nvm, I totally forgot that the temproal_id was never assigned to VP9 and AV1 with the previous logic. I'll restrict it to H264
Related details
- Eugene Zemtsov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan voted and added 4 comments![]( "Open in Gerrit")
Votes added by Phil Yan
| Commit-Queue | +0 |
4 comments
TemporalScalabilityIdExtractor::BitstreamMetadata bits_md;
if (!svc_parser_->ParseChunk(out_buffer_data, input_since_keyframe_count_,
bits_md)) {
NotifyErrorStatus({EncoderStatus::Codes::kEncoderHardwareDriverError,
"Parse bitstream failed"});
return;
}
temporal_id = bits_md.temporal_id;Phil Yanlet's do this only for H264
Phil Yanplease correct me if I misunderstood - if we also execute this fallback path, which is the existing temporal id retrieval logic, for H264, won't AV1 and VP9 be affected before the native SVC layer id support is enabled? Or do you mean we only assign the temporal id for H264 in the switch statement below?
Phil Yantypo: if we *only* execute this fallback path...
oh nvm, I totally forgot that the temproal_id was never assigned to VP9 and AV1 with the previous logic. I'll restrict it to H264
Done. Also slightly updated the logic flow as we not expect the native id retrieval for AV1 and VP9 to get a valid id when it's supported; otherwise, signal the error and return early.
static constexpr char AMEDIAFORMAT_KEY_TEMPORAL_LAYER_ID[] =Phil Yanstring_view?
Done
move this check behind the version check
Done
return base::android::android_info::sdk_int() >= 37;Phil Yanprefer `__builtin_available(android 37, *)`
Done
Related details
- Eugene Zemtsov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan added 1 comment![]( "Open in Gerrit")
static constexpr char AMEDIAFORMAT_KEY_TEMPORAL_LAYER_ID[] =Phil Yanstring_view?
Done
One a second thought, would using char* be better here as AMediaFormat_setInt32 and AMediaFormat_getInt32 expect char*. When using string_view, we'd need to call .data() on it which is not guaranteed to be null-terminated?
Related details
- Eugene Zemtsov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eugene Zemtsov (Gerrit)
Eugene Zemtsov added 2 comments![]( "Open in Gerrit")
// If native retrieval was expected but failed, treat it as a hardwarethis comment doesn't look correct wrt vp9 and av1
static constexpr char AMEDIAFORMAT_KEY_TEMPORAL_LAYER_ID[] =Phil Yanstring_view?
Phil YanDone
One a second thought, would using char* be better here as AMediaFormat_setInt32 and AMediaFormat_getInt32 expect char*. When using string_view, we'd need to call .data() on it which is not guaranteed to be null-terminated?
got it. we're going to replace it all with NDK consts so nocities don't really matter.
Related details
- Phil Yan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan added 1 comment![]( "Open in Gerrit")
// If native retrieval was expected but failed, treat it as a hardwarethis comment doesn't look correct wrt vp9 and av1
You're right. IIUC, what follow_svc_spec = true does is actually is asking the downstream to check the bitstream header for the layer id info.
Related details
- Eugene Zemtsov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eugene Zemtsov (Gerrit)
Eugene Zemtsov voted Code-Review+1![]( "Open in Gerrit")
- Phil Yan
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phil Yan (Gerrit)
Phil Yan voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[NdkVEA] Implement native temporal layer ID retrieval.
This CL implements native temporal layer ID retrieval in
NdkVideoEncodeAccelerator using the AMEDIAFORMAT_KEY_TEMPORAL_LAYER_ID
key which is available starting from Android API 37.
Key changes:
- Added kNdkVideoEncodeAcceleratorNativeSvc feature flag.
- The flag is necessary because even for Android 17 (API 37), some
development branches may not have full support for the new temporal
layer encoding API yet. This allows us to guard the new logic and
ensure stability.
- Updated CreateVideoFormat to signal that we want the layer ID in
the output format.
- Modified DrainOutput to try native retrieval first, with a fallback
to the existing bitstream parsing for H.264 if native retrieval is
not available or the flag is disabled.
- Updated metadata assignment: H.264 continues to use .h264 structure,
while VP9 and AV1 now use .svc_generic structure when the flag is
enabled.
- M media/base/media_switches.cc
- M media/base/media_switches.h
- M media/gpu/android/ndk_video_encode_accelerator.cc
- M media/gpu/android/ndk_video_encode_accelerator_svc_api.cc
- M media/gpu/android/ndk_video_encode_accelerator_svc_api.h
Code-Review: +1 by Eugene Zemtsov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |