aws / aws/aws-cdi-sdk

Unexpected behavior (and some issues) in the baseline profile APIs

Open
#99 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
63
Forks
19
PR merge metrics
No merged PRs in 30d

Description

I've recently been exploring extensibility of the CDI baseline profile and observe behavior that I find unexpected, at least, given my interpretation of the information found [here](https://aws.github.io/aws-cdi-sdk/mainline/AVM_home_page.html) and that I partially reproduce below.

> Extensibility is addressed through a generic configuration mechanism which is used even for the CDI baseline profile. It is based on a structure containing a URI and optional parameter data. The URI is defined such that it ensures uniqueness and optionally points to documentation on how to interpret the parameter data. The format of the payload data is also dependent on the URI. Helper functions ease the process of creating and parsing the generic configuration structure for the CDI baseline profile.
>
> The URIs used for the CDI baseline profile are:
>
> https://cdi.elemental.com/specs/baseline-video
> https://cdi.elemental.com/specs/baseline-audio
> https://cdi.elemental.com/specs/baseline-ancillary-data
>
> The documents at those URIs fully specify the various aspects of each media type including the parameter data and the in memory representation of payload data. These files also reside in CDI_SDK/doc/specs.
>
> Since the media format details are specificated outside of the SDK, new formats (beyond the CDI baseline profile) can be added without changing the SDK. They can be publicly documented or they can remain private for situations where interoperability is not required.

My initial understanding is that a transmitter can, for example, define a custom video format by calling `CdiAvmRegisterBaselineProfile` with a media type of `kCdiAvmVideo` and at some point, use `CdiAvmMakeBaselineConfiguration2` with a custom video configuration derived from `CdiAvmBaselineConfigCommon` to initialize an AVM generic configuration structure. This will result in the `make_config_ptr` function specified by the custom profile being called where it presumably needs to set the `uri` member of the configuration to **something other than "https://cdi.elemental.com/specs/baseline-video"**.

On the receiver side, `CdiAvmRegisterBaselineProfile` must also be called in order to handle this custom video format so that it can convert the generic configuration structure back to the custom video configuration by calling `CdiAvmParseBaselineConfiguration2`. However, this currently fails with a status of `kCdiStatusNonFatal` as the function validates the `uri` member and **only allows URIs for the built-in payload types**.

https://github.com/aws/aws-cdi-sdk/blob/89d21b0d1ae842498b850f34188c821c981b2819/src/cdi/baseline_profile.c#L285-L287

This is unexpected. Setting the URI as `https://cdi.elemental.com/specs/baseline-video` instead does work, but this seems to go against what seems to be implied by the extensibility description above.

There's also a second aspect that isn't entirely clear. The `uri` member of the `CdiAvmConfig` structure is documented as follows. Note in particular the reference to "**or other**" when describing the payload types, implying that it should be possible to register a media type other than video, audio, or ancillary data.

> The URI unambiguously specifies the type (audio, video, ancillary data, or other) of data comprising an AVM stream within an AVM connection. Typically, it will be a URL to a document that describes how to interpret the bytes of the enclosing structure's data member as well as how payload data is to be formatted.

On the other hand, the `CdiAvmRegisterBaselineProfile` must be called with a `media_type` parameter with one of the values in the `CdiBaselineAvmPayloadType` enumeration and it's unclear how to register a custom media type. For example, calling this function with a `kCdiAvmNotBaseline` media type isn't allowed and will return a result of `kCdiStatusInvalidParameter` whereas using a value not defined by `CdiBaselineAvmPayloadType` is "allowed" but, currently, as the parameter isn't validated, it will result in data corruption when the profile data is written beyond the boundaries of the `baseline_profile_array`.

From the above, it seems that the implementation still relies on many aspects of the non-extensible API and that it would have been more appropriate for the `media_type` parameter to have been a URI instead of a `CdiBaselineAvmPayloadType`.

Finally, there's a discrepancy between the behavior of `CdiAvmKeyEnumToString` (and probably other similar functions) and what is currently documented, where it states that it will use profile version 01.00 if the `version_ptr` parameter is NULL. The actual behavior is to return the first **registered** profile which will not necessarily be v1.00 if `CdiAvmRegisterBaselineProfile` has been called previously.

https://github.com/aws/aws-cdi-sdk/blob/57ce52b0dfd68a6d2bea3a9e397886c801c28b08/include/cdi_baseline_profile_api.h#L225-L235

In fact, there seems to be a deeper issue in the manner in which the baseline profiles are set up since this occurs in the `FindProfileVersion` function where once `InitializeBaselineProfiles` is called, it sets the `initialized` flag.

https://github.com/aws/aws-cdi-sdk/blob/57ce52b0dfd68a6d2bea3a9e397886c801c28b08/src/cdi/baseline_profile.c#L154-L162

Because `CdiAvmRegisterBaselineProfile` does **NOT** call `FindProfileVersion` unless the `initialized` flag has been set, it might register the specified profile as the first one resulting in behavior that can change depending on whether any other function that also depends on `FindProfileVersion` has been called previously. Moreover, the check for duplicate registrations and the call to `FindProfileVersion` itself is skipped when the baseline profiles have not been initialized allowing, for example, to register the same profile multiple times or even registering alternate v1.0 or v2.0 profiles that will override those provided by the SDK.

https://github.com/aws/aws-cdi-sdk/blob/57ce52b0dfd68a6d2bea3a9e397886c801c28b08/src/cdi/baseline_profile.c#L214-L216

Contributor guide

Open the contributing guide

Research direction

Start with src/cdi/baseline_profile.c, especially the URI validation, FindProfileVersion, InitializeBaselineProfiles, and registration paths, then compare them with the declarations and documentation in include/cdi_baseline_profile_api.h. Trace custom URI handling, media-type registration, profile initialization order, duplicate registration, and NULL version selection. Done means the intended extensibility and registration behavior is clarified and the documented inconsistencies are addressed without allowing invalid profile data.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
api, backend-api-design
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.