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

ALE adapter round-trip CDL Values #1050

Open
reinecke opened this issue Aug 24, 2021 · 2 comments · May be fixed by #1054
Open

ALE adapter round-trip CDL Values #1050

reinecke opened this issue Aug 24, 2021 · 2 comments · May be fixed by #1054
Assignees
Labels
enhancement A request for something new. good first issue If you're looking for a way to contribute, but not sure where to start. This is a good first issue.

Comments

@reinecke
Copy link
Collaborator

Feature Request

New feature

Description

The ALE adapter reads CDL values into the cdl namespaces of clip metadata dictionaries from the ASC_SAT and ASC_SOP fields, however it does not write them back out. It would be helpful to have the ALE adapter round-trip this data. This would also match the EDL adapter behavior.

Context

For how the adapter reads CDL values, look here.
For an example of how the EDL adapter writes out this metadata, look here

@reinecke reinecke added enhancement A request for something new. good first issue If you're looking for a way to contribute, but not sure where to start. This is a good first issue. labels Aug 24, 2021
@Viraj-Rana008
Copy link
Contributor

Viraj-Rana008 commented Aug 29, 2021

@reinecke I want to work on this enhancement.
So what we need is to append the ASC_SAT and ASC_SOP values in the line list (of _parse_data_line function). In the same format as done in the EDL adapter. Right?

@reinecke
Copy link
Collaborator Author

Hi @Viraj-Rana008 Thanks for your interest!
I'd look at the write_to_string function in the adapter.

Note that at the moment, the adapter builds a list of columns that are in the ALE metadata to determine what to output, it also gathers some "special" column names that it will always output.

My approach would be to add some code to detect if the cdl key is present in the clip metadata and add ASC_SOP and ASC_SAT columns if at least one of the clips has it. Then, in val_for_column, I would add handling for generating the ale formatted versions of those.

In the unittests, sample_cdl.ale is a good example of how ALE formats those fields.

Additional note: sample_cdl.ale has a CDL column in it. I'm not sure what software generates that, all the CDL examples I have on-hand use the ASC_SOP and ASC_SAT columns. I see in the parsing code that any values in CDL are overridden with values from ASC_SOP and ASC_SAT - this implies that ASC_SOP and ASC_SAT are considered to be the source of truth.
I think the best idea for us when writing is to adhere to the ASC_SOP and ASC_SAT columns. If someone has a use case for writing a CDL column, we can consider that as a separate request.

@reinecke reinecke linked a pull request Sep 2, 2021 that will close this issue
@jminor jminor linked a pull request Sep 2, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A request for something new. good first issue If you're looking for a way to contribute, but not sure where to start. This is a good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants