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

Cannot set arbitrary (unsupported) tags in Vorbis Comment? #43

Closed
jchennales opened this issue Dec 31, 2018 · 17 comments
Closed

Cannot set arbitrary (unsupported) tags in Vorbis Comment? #43

jchennales opened this issue Dec 31, 2018 · 17 comments
Assignees

Comments

@jchennales
Copy link

tageditor can properly list all the fields in Vorbis Comment when given the -u switch. I am not seeing an equivalent for set, where we should be able to set any key / field name whether tageditor undesstands its purpose or not. Maybe implementing the same -u switch for set and avoiding the "generic field name is unknown" error.

I think this has merit on its own, but in particular, I am interested in setting "ALBUMARTIST" field which is used by many players to represent a single artist for a whole compilation disk, like a DJ's name for a set etc. This is similar to but probably not the same as "grouping". See this link https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping

Perhaps you are also interested in supporting this field specifically too.

Thanks for writing this useful program!

@Martchus
Copy link
Owner

Setting unsupported tags is actually implemented and documented in the README.

I'll add support for album artist. Should be a 5 minute task :-)

@Martchus
Copy link
Owner

Martchus commented Jan 1, 2019

I added support for "ALBUMARTIST" (not much tested, though):

@Martchus Martchus self-assigned this Jan 1, 2019
@jchennales
Copy link
Author

Hi, Happy new year!

Thanks for replying so promptly. Sorry, I had misread the readme regarding the unsupported fields. Looking at it again based on your reply, I tried to use the vorbis:ALBUMARTIST and the program gives no error but also sets no new field:

`$ tageditor get -u -f rhythm.opus
Tag information for "rhythm.opus":

  • Vorbis comment (in Opus stream)
    Title Rhythm Bar
    Album Silly Sounds With No Copyright
    Artist Audacity
    Genre Sound Clip
    Year 2019
    Comment Test file created to meddle with tags
    Track 1
    Encoder Lavc57.107.100 libopus
    Description Test file created to meddle with tags

$ tageditor set vorbis:ALBUMARTIST="CrazyDJ" -f rhythm.opus
Setting tag information for "rhythm.opus" ...

  • [100%] Prepare for rewriting OGG file ...
  • Changes have been applied.

$ tageditor get -u -f rhythm.opus
Tag information for "rhythm.opus":

  • Vorbis comment (in Opus stream)
    Title Rhythm Bar
    Album Silly Sounds With No Copyright
    Artist Audacity
    Genre Sound Clip
    Year 2019
    Comment Test file created to meddle with tags
    Track 1
    Encoder Lavc57.107.100 libopus
    Description Test file created to meddle with tags

$ grep -il ALBUMARTIST rhythm.opus

$ grep -il ARTIST rhythm.opus
rhythm.opus
`

I am attaching this simple rhythm.opus file I generated (it's just a beat bar from audacity)

rhythm.zip

BTW: If you are interested in fixing typos in the documentation, this section about unsupported fields uses "file.mkv" and "file.m4a" in the example and then mentions them as test.mkv and test.m4a. :)

@jchennales
Copy link
Author

I added support for "ALBUMARTIST" (not much tested, though):

* [Martchus/tagparser@6afcd0f](https://github.com/Martchus/tagparser/commit/6afcd0f8d33bcc6a1ebaebff4806883f8895df7f)

* [dbc1c79](https://github.com/Martchus/tageditor/commit/dbc1c79f4961c213ec70e4e4869087864f7cfc9b)

If you have a binary available I'd be happy to do the testing.

@Martchus
Copy link
Owner

Martchus commented Jan 2, 2019

I just had a brief look at my testcases and indeed one for vorbis:... is missing. So likely it is broken for Vorbis Comments right now. (I have already a suspicion what's the problem.)

I assume you would need a Windows binary?

@jchennales
Copy link
Author

I would personally need a Windows binary, yes. I can help by testing on a Linux VM just for the sake of helping if that is needed.

@Martchus
Copy link
Owner

Martchus commented Jan 2, 2019

I've tested it myself and it seems to work. I've also fixed the problem with setting the custom field (only affected Vorbis Comments in Ogg).

I can provide a binary with the fixes when I'm again on my machine with the mingw-64 toolchain installed.

@Martchus
Copy link
Owner

Martchus commented Jan 4, 2019

@jchennales
Copy link
Author

Sorry, but this gives me "Segmentation Fault" when I run it with my cygwin bash shell or no message at all when I run it in CMD. I am running "Microsoft Windows [Version 10.0.17134.472]" (64 bit Pro)
I tried on two different systems but both running the same Windows though. I can probably get my hands on a Win7 x64 machine if you feel it is worth trying.

@Martchus
Copy link
Owner

Martchus commented Jan 4, 2019

I only tested with WINE and it worked. Maybe I misconfigured the build. You can test on Windows 7 if you like but actually it is supposed to work under Windows 10 as well.

@Martchus
Copy link
Owner

Martchus commented Jan 5, 2019

I was lately playing with compiler flags used for Windows builds. Apparently one of those flags didn't work that well (see commit message Martchus/PKGBUILDs@f017587 for details).

So sorry for the inconvenience. But after all it is a developer build :-)
I replaced the binary on my server. You can try to download it again.

@jchennales
Copy link
Author

Hey, no complaints at all... you are doing all the work ;)

This last binary works. I can set albumartist as a supported tag and also set arbitrary vorbis tags too. There is still a glitch with the "supported" status of albumartist. It is set fine but it is not listed back, even with the -u switch which then does show the other arbitrary tags I tried. The funny thing is that once the albumartist tag is set, the release build of tageditor CAN show albumartist with the -u switch (as it always could if set by a third party tagger). So evidently it is stuck in some semi-supported state where the program knows about it but it is forgetting to show it (possibly with its mapped name).

Hope this description helps track it. Thanks!

@Martchus
Copy link
Owner

Martchus commented Jan 5, 2019

Fixed the semi-supported state (absolutely right observation by the way :-) ).

I also added documentation how to add new fields (so I don't forget some of the required steps the next time). It is actually quite easy and does not really require any C++ knowledge. So in case you need support for more fields, here you go: https://github.com/Martchus/tagparser/blob/master/doc/adding-new-fields.md

@jchennales
Copy link
Author

Hi, how are you? Do you think you could build the win binary again with these fixes? I see the last one from the link you sent is still the one from Jan 5. Thanks!

@Martchus
Copy link
Owner

Martchus commented Jan 8, 2019

Maybe I should create Windows development builds in the same way as releases and Linux development builds. But I suppose I'll create the next release soon anyways.

@Martchus
Copy link
Owner

Martchus commented Jan 9, 2019

Here's the (possibly) next release build: https://martchus.no-ip.biz/repo/arch/ownstuff-experimental/os/x86_64/mingw-w64-tageditor-static-3.1.3-1-any.pkg.tar.xz

By the way, I'm fine - thanks for asking :-)

@Martchus
Copy link
Owner

The fixed version is now released as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants