BWFMetaEdit icon indicating copy to clipboard operation
BWFMetaEdit copied to clipboard

Option to remove specific chunks

Open MarcosSueiro opened this issue 2 years ago • 11 comments

Follow up to #163

Add option to remove specific chunks: --remove-chunks=[NAME]

MarcosSueiro avatar Feb 06 '24 20:02 MarcosSueiro

@MarcosSueiro please test this snapshot with --remove-chunks= option.

JeromeMartinez avatar Feb 21 '24 16:02 JeromeMartinez

I will test tomorrow since I am working from home. I am very excited, thank you so much!!

Marcos Sueiro Bal Archive Manager 646 829 4063 From: Jérôme Martinez @.> Sent: Wednesday, February 21, 2024 11:01 AM To: MediaArea/BWFMetaEdit @.> Cc: Marcos Sueiro Bal @.>; Mention @.> Subject: Re: [MediaArea/BWFMetaEdit] Option to remove specific chunks (Issue #281)

@MarcosSueirohttps://github.com/MarcosSueiro please test this snapshothttps://mediaarea.net/download/snapshots/binary/bwfmetaedit/20240220/ with --remove-chunks= option.

— Reply to this email directly, view it on GitHubhttps://github.com/MediaArea/BWFMetaEdit/issues/281#issuecomment-1957087611, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAD7LVD22CY2QHUOOEZTKC3YUYK4VAVCNFSM6AAAAABC4VABZOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJXGA4DONRRGE. You are receiving this because you were mentioned.Message ID: @.@.>>

MarcosSueiro avatar Feb 29 '24 18:02 MarcosSueiro

Thank you so much for implementing this! Please see results below.

I tested --remove-chunk= with two files:

  1. File 1 has one 'levl' chunk, 54 'mtyp' and 'labl' subchunks under the 'adtl' chunk, and an 'r64m' chunk
  2. File 2 has one chunk of each of the following labels: 'fact', 'mext', 'levl', 'aux '

image

  • First test: remove 'r64m' chunk from File 1 --successful
  • Second test: remove 'levl' chunk from both files, while the files are open in GUI --successful (needed to reload to see results in GUI)
  • Third test: remove 54 'mtyp' chunks from File 1 --unsuccessful (although it is no longer listed in table view, Trace view shows chunks)
  • Fourth test: remove 54 'labl' chunks from File 1 --unsuccessful (although it is no longer listed in table view, Trace view shows chunks)
  • Fifth test: try/catch remove 'adtl' chunk from File 1 --unsuccessful (i.e., BWFME removed a supported chunk)
  • Sixth test: remove chunks 'fact', 'mext', 'levl' (not present), 'aux ' using --remove-chunks=fact,mext,levl,aux from File 2 --unsuccessful (could not identify 'aux ' chunk)
  • Seventh test: remove chunks 'fact', 'mext', 'levl' (not present), 'aux ' using --remove-chunks=fact,mext,levl,aux_ from File 2 --partially successful (removed all chunks except 'aux ' chunk)
  • Eighth test: remove 'aux' chunk from File 2 using --remove-chunks=aux ,levl or --remove-chunks=aux_,levl --unsuccessful (BWFME did not remove aux chunk)

MarcosSueiro avatar Mar 05 '24 20:03 MarcosSueiro

Fifth test: try/catch remove 'adtl' chunk from File 1 --unsuccessful (i.e., BWFME removed a supported chunk)

We don't prevent the removal of supported chunks on purpose, as an easy way to remove this kind of chunks, no difference between supported and unsupported chunks, so IMO it is fine.

Note that we block the removal of mandatory chunks e.g. "fmt "

[...] aux [...]

Works for me with a fake file (I have no real file with "aux "

$ mediainfo --Details=1 a.WAV | grep Name | grep "aux "
0017F8    Name:                                aux
$ bwfmetaedit "--remove-chunks=aux ,levl" a.WAV
$ mediainfo --Details=1 a.WAV | grep Name | grep "aux "
$

Did you think to quote the option in order to provide correctly the space to the program? Note that we don't convert "_" to " ", so if your file has a space you can not use an underscore.

[...] mtyp [...] labl [...]

Tried the same way (without quote) with fake files and it is fine. and weird that this is still in trace but not in table view, did you reload files? Please share non working files.

(needed to reload to see results in GUI)

The GUI does not detect a file change and does not reload, a feature we need to add, another topic.

JeromeMartinez avatar Mar 05 '24 21:03 JeromeMartinez

  • Ninth test: remove 'aux' chunk from File 2 using "--remove-chunks=aux " (with quotes) --successful

File with multiple mytp and labl subchunks (Jerome only) here

MarcosSueiro avatar Mar 05 '24 21:03 MarcosSueiro

We don't prevent the removal of supported chunks on purpose, as an easy way to remove this kind of chunks, no difference between supported and unsupported chunks, so IMO it is fine.

Since this is potentially dangerous, could we add a warning sign before deleting supported chunks?

MarcosSueiro avatar Mar 05 '24 21:03 MarcosSueiro

Since this is potentially dangerous, could we add a warning sign before deleting supported chunks?

I have mixed feelings about that, an future issue I see is that a new version of the tool supporting a new chunk item would reject the command, so not responding as the previous version, it seems dangerous to me. Do you think that this potential behavior change between versions is acceptable?

And how dangerous is it, as the user explicitly indicate that this item should be removed?

File with multiple mytp and labl subchunks

I see, they are not at the top level, and currently we support only top level items. cc @g-maxime IMO the interface will be something like "--remove-chunks=cue /adtl/mtyp" in order to avoid conflict with top level chunks with the same name. And another limit is that we won't support sub-chunks of unsupported chunks (technically we could, but more work).

JeromeMartinez avatar Mar 06 '24 14:03 JeromeMartinez

I have mixed feelings about that, an future issue I see is that a new version of the tool supporting a new chunk item would reject the command, so not responding as the previous version, it seems dangerous to me. Do you think that this potential behavior change between versions is acceptable?

This is what I see as a potential problem:

Some of our files have (as you have seen) many types of extraneous chunks. Let's say one of the extraneous chunks is called 'adtk', but my finger slips and I write 'adtl' instead of 'adtk': --remove-chunks=levl,adtk,mtyp.

If BWFME does not warn me, I would remove a chunk that I did not intend to delete.

As I see it, we could do two approaches (I think I prefer the first one):

  1. Always generate a warning, as follows:
ATTENTION! You are about to remove the following chunks:
 - adtl (** CURRENTLY SUPPORTED in Version 24.01, for Windows **)
 - levl (currently unsupported)
 - mtyp (currently unsupported)

Please keep in mind that:
* Removing chunks may affect compatibility with other software
* Different versions of BWF MetaEdit may support different chunks 
--see https://mediaarea.net/BWFMetaEdit for details
Do you wish to proceed (y/n)?
  1. Generate a warning only for currently supported chunks:
ATTENTION! You are about to remove the following supported chunks
in BWFME Version 24.01, for Windows:
 - adtl 
 
Please keep in mind that:
* Removing chunks may affect compatibility with other software
* Different versions of BWF MetaEdit may support different chunks 
--see https://mediaarea.net/BWFMetaEdit for details
Do you wish to proceed (y/n)?

I see, they are not at the top level, and currently we support only top level items. cc @g-maxime IMO the interface will be something like "--remove-chunks=cue /adtl/mtyp" in order to avoid conflict with top level chunks with the same name. And another limit is that we won't support sub-chunks of unsupported chunks (technically we could, but more work).

I like this solution! And I think supporting sub-chunks of unsupported chunks would be too confusing. 😵‍💫

MarcosSueiro avatar Mar 06 '24 15:03 MarcosSueiro

Hello, the link to the snapshot that includes --remove-chunks is dead, could you re-post?

https://mediaarea.net/download/snapshots/binary/bwfmetaedit/20240220/

MarcosSueiro avatar Sep 06 '24 13:09 MarcosSueiro

Hello, the link to the snapshot that includes --remove-chunks is dead, could you re-post?

https://mediaarea.net/download/snapshots/binary/bwfmetaedit/20240220/

https://mediaarea.net/download/snapshots/binary/bwfmetaedit/20240906-3/

g-maxime avatar Sep 06 '24 23:09 g-maxime

Thank you!

MarcosSueiro avatar Sep 16 '24 12:09 MarcosSueiro