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: add json output format support #6

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

nipierre
Copy link
Contributor

  • Add json output support for more thorough exploitation of data

@nipierre nipierre requested a review from valnoel March 31, 2021 12:54
match input {
"EBU_TT_D" => Ok(OutputFormat::EbuTtD),
"JSON" => Ok(OutputFormat::Json),
_ => Ok(OutputFormat::EbuTtD),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should log a warning message, to notify the specified format is not supported, don't you think?

@@ -89,6 +113,9 @@ impl MessageEvent<WorkerParameters> for TranscriptEvent {
let selected_streams = get_first_audio_stream_id(&format_context)?;

let cloned_parameters = parameters;
let param_output_format = cloned_parameters.output_format.clone();

let output_format = OutputFormat::from_str(&(param_output_format.unwrap_or("JSON".to_string()))).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the default output format is JSON here but EBU_TT_D at line 81?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also handle the error instead of "unwrap()"

OutputFormat::Json => {
let sequence_index = sequence_number.load(Acquire);
let message = serde_json::to_string(&event).unwrap_or("".to_string());
info!("{:?}", message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logged message could be more explicit, don't you think? :)

destination_path: String,
source_path: String,
}

#[derive(Debug, Clone, Deserialize, JsonSchema)]
enum OutputFormat {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you define it into a specific file? To lighten the main file...

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.

2 participants