search

MediaArea / BWFMetaEdit

WAV/BWF metadata editor
https://MediaArea.net/BWFMetaEdit
Other
51 stars 19 forks source link

CodingHistory rules do not recognize Frequency parameter #251

Closed EricJacobsTAA closed 1 year ago

EricJacobsTAA commented 1 year ago

BWF MetaEdit 21.07 (MacOS 10.13.6)

BEXT Chunk - CodingHistory rules do not allow or recognize Frequency parameter "F="

Allowed: A=PCM,W=24,M=stereo,T=Mytek Stereo192;A/D

Malformed Message: A=PCM,F=96000,W=24,M=stereo,T=Mytek Stereo192;A/D

JeromeMartinez commented 1 year ago

96000 is not valid with EBU R98-1999. but this is a so old spec... Nowadays 96k is common, and FADGI recommendation accepts it. @kmurmur I am wondering if it isn't worth it to accept frequencies >48kHz, currently they are accepted with FADGI recommendations but not EBU R98, should we be very strict or accept technical evolution (I am in favor of that, as there is no other way to signal 96k) when EBU R98 is selected?

kmurmur commented 1 year ago

@JeromeMartinez I think if R98 is selected, then it needs to conform to the published version. As you say, R98 is very old. It's been suggested that FADGI work with EBU to update it and we can pursue that (potentially) but it takes time. Meanwhile however, I think if a user is using R98 validation rules, then one has to work with what is defined in R98.

EricJacobsTAA commented 1 year ago

This is via the GUI, I did not test the CLI. I've not tested this yet with CSV automation.

No value of "F=" will pass validation with or without the CodingHistory Rule enabled via the GUI.

The issue is not a matter of EBU R98-1999 strictness, although in hindsight I probably should have used a different example.

I get a malformed flag for any value of Frequency, including "F=48000", "F=44100" (again, I get the malformed error with and without the CodingHistory Rule check).

Sorry for the confusion.

On the matter of EBU R98 strictness, "malformed" (the word in the error message) implies a syntax check (missing characters, invalid char, etc.).

If the syntax is correct, but the value is invalid, that would seem to be a different error (e.g. "invalid value") and would be clearer to the end-user.

With the exception of "F=" (any value), the CodingHistory parser passes all of my multi-process cases with no difficulty.

On Tue, Mar 7, 2023 at 6:34 AM Jérôme Martinez @.***> wrote:

96000 is not valid with EBU R98-1999 https://tech.ebu.ch/docs/r/r098.pdf. but this is a so old spec... Nowadays 96k is common, and FADGI recommendation accepts it. @kmurmur https://github.com/kmurmur I am wondering if it isn't worth it to accept frequencies >48kHz, currently they are accepted with FADGI recommendations but not EBU R98, should we be very strict or accept technical evolution (I am in favor of that, as there is no other way to signal 96k) when EBU R98 is selected?

— Reply to this email directly, view it on GitHub https://github.com/MediaArea/BWFMetaEdit/issues/251#issuecomment-1458280677, or unsubscribe https://github.com/notifications/unsubscribe-auth/A6KFYXAEIY44XV7ON4FFIWLW25BO5ANCNFSM6AAAAAAVR27ZBU . You are receiving this because you authored the thread.Message ID: @.***>

EricJacobsTAA commented 1 year ago

TL;DR - can we implement the EBU R98-1999 CodingHistory Rule (abbreviating this as "CHR98") for "F=value", the same way as "W=value"?

Current implementation for "W=value"

-- CHR98 enabled will not allow "W=32" (red flag in GUI, "malformed" message) -- CHR98 disabled will allow "W=32" (no flag in GUI, no message)

This is good enough!

BUG - CHR98 enabled or disabled does not allow "F=any valid value"

It is currently not possible to enter "F=any valid value" in GUI (with or without CHR98 enabled) without getting a "malformed" error. This is a real problem, as sampling, upsampling and downsampling processes cannot be captured in CodingHistory.

BUG FIX (SUGGESTION) - handle "F=value > 48000" like "W=value" when CHR98 enabled and disabled

-- CHR98 enabled, do not allow "F=96000" (red flag in GUI, "malformed" message) -- CHR98 disabled, allow "F=96000" (no flag in GUI, no message)

BUG FIX (NICE TO HAVE): enhance errors and warnings to promote use of CodingHistory:

-- distinguish between "malformed" syntax errors (e.g. missing commas, missing CRLF, comma before CRLF, etc.) versus "invalid" value errors (e.g. F=96000, W=32, etc.) -- display RED ERROR flags in the GUI for CHR98 enabled versus YELLOW WARNING flags for CHR98 disabled

Example:

-- CHR98 enabled, do not allow "F=96000" (red flag in GUI, "invalid value" error message) -- CHR98 disabled, allow "F=96000" (yellow flag in GUI, "invalid value" warning message)

THOUGHTS on technical evolution

My personal preference allows for both strict checking and technical evolution:

-- CHR98 enabled - strict checking with ERRORs --- disallow saving if ANY errors - RED flag in GUI

-- CHR98 disabled - strict checking with ERRORs and WARNINGs --- disallow saving invalid SYNTAX (ERROR) to support consistency and comprehension over time - RED flag in GUI --- allow saving invalid VALUES (WARNING) for technical evolution - YELLOW flag in GUI

g-maxime commented 1 year ago

No value of "F=" will pass validation with or without the CodingHistory Rule enabled via the GUI.

Ok, let's check if I test it correctly: 1st way: right click, then paste "A=PCM,F=96000,W=24,M=stereo,T=Mytek Stereo192;A/D" on the Coding history column in the core view -> No validation error.

2st way: Open the coding history dialog from the File view, go to freetext tab, paste and close the dialog -> No validation error.

JeromeMartinez commented 1 year ago

This is a real problem, as sampling, upsampling and downsampling processes cannot be captured in CodingHistory.

Do you mean a value not in the list (96k included)? I tested with 96k and it is fine (without CHR98). This is a separate issue. And we can tweak the editor for being more flexible.

-- CHR98 disabled - strict checking with ERRORs and WARNINGs

If disabled, we show no error or warning, because it is linked to CHR98 enabled.

enhance errors and warnings to promote use of CodingHistory:

@kmurmur would you be fine with that? e.g. due to the past, with different guidelines, we implement an intermediate level "warning", with an offer to normalize files. but... We need to decide about what is a warning and what is an error.

Actually, we need to decide first what is the long term target.

Note that we may also enforce an unique choice by default and also add options for letting the user explicitly accept another choice.

EricJacobsTAA commented 1 year ago

@JeromeMartinez My error and my deepest apologies - F=96000 works correctly. I'm sorry to have caused you to reproduce my flawed test.

My best guess is that I had a comma-CRLF from a past test while testing the "F=96000" case. The CodingHistory pane is relatively small and it's difficult to view the entire line - I think that is what happened.

I re-ran my tests with an updated test matrix (see below).

UPDATED TEST MATRIX

FADGI enabled A=ANALOG with CHR98 enabled - error (CONFLICT? EBU && FADGI enabled, which takes precedence?) A=ANALOG with CHR98 disabled - error (NOT GOOD, FADGI only, but FADGI allows A=ANALOG)

FADGI disabled A=ANALOG with CHR98 enabled - error (GOOD, EBU only) A=ANALOG with CHR98 disabled - no error (GOOD, both FADGI and EBU disabled)

F=96000 with CHR98 enabled - error (GOOD) F=96000 with CHR98 disabled - no error (GOOD)

W=32 with CHR98 enabled - error (GOOD) W=32 with CHR98 disabled - no error (GOOD)

comma-CRLF with CHR98 enabled - error (UNCLEAR, EBU R98 examples ambiguous) comma-CRLF with CHR98 disabled - error (UNCLEAR, EBU R98 examples ambiguous)

FADGI vs EBU PRECEDENCE

When FADGI and EBU conflict, it might be helpful to have some clarity on precedence.

Clarity in rule selection? -- more granular rule options, such as specific checks (e.g. "allow comma-CRLF", "allow F>48000", "allow A=ANALOG")? -- Radio buttons to prevent FADGI && EBU conflicts?

Clarity in error/warning messages? -- which rule was followed ("A=ANALOG not allowed (EBU R98-1999)")

EXCEPTIONS - COMMA-CRLF ETC

I think a good case can be made to allow comma-CRLF with CHR98 enabled. Comma-CRLF appears in the original EBU R98 examples for CodingHistory if we are being strict. EBU R98 doesn't make any distinction about when to use or not use the comma.

Maybe add specific rules (more explicit)? "[ ] BWF CodingHistory (EBU Tech R98-1999) - allow comma-CRLF" "[ ] BWF CodingHistory (EBU Tech R98-1999) - allow A=ANALOG" "[ ] BWF CodingHistory (EBU Tech R98-1999) - allow F>48000" "[ ] BWF CodingHistory (EBU Tech R98-1999) - allow W>24"

I dislike complicated GUIs, especially with options that might overwhelm novice users. But clarity and control are important, too, especially for experts.

Too what extent do you want the Rules to be a blackbox?

WORKFLOW

We are heavy users of CodingHistory, and strongly advocate its value and use. We use CodingHistory for full traceability at the machine serial number level, as well as software revisions as part of our QC, diagnostics, and maintenance programs. I don't know how many organizations have legacy files with CodingHistory, and among those how many are "strict". We have many thousands of legacy files spanning nearly 20 years, all with comma-CRLF (we chose to follow the EBU R98 "Example 2" for analog material). Each CodingHistory contains 2-4 process lines. Without exception handling, the CodingHistory Rules are not very useful in our case since every file will flag an error. Once a project is completed, we continue to run regular MD5 checksums for integrity checking, estimate individual machine operation time for maintenance, and identify files impacted by software upgrades.

JeromeMartinez commented 1 year ago

A=ANALOG with CHR98 enabled - error (CONFLICT? EBU && FADGI enabled, which takes precedence?)

In theory there should be no conflict, and @kmurmur indicated that ANALOG was a mistake, so we have to decide about what is good or not good (or in between i.e. acceptable but not recommended) then we'll manage such case.

Maybe add specific rules (more explicit)?

We'll do a test with that and see what we decide with @kmurmur.

WORKFLOW [...]

Thank you for the feedback, the project is intended for all so we need to find a good balance between specs, recommendations, past, and user needs.

JeromeMartinez commented 1 year ago

@EricJacobsTAA please test latest development snapshots.

EricJacobsTAA commented 1 year ago

@JeromeMartinez thank you for all your hard work on sorting this out. How do I test? Is there a test build? I don't know how to do a build (MacOS or Windows).

JeromeMartinez commented 1 year ago

I don't know how to do a build (MacOS or Windows).

Direct links : Windows, macOS.