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

Add, change and fix some mappings #544

Merged
merged 8 commits into from Jul 6, 2020
Merged

Add, change and fix some mappings #544

merged 8 commits into from Jul 6, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 21, 2020

  • category: Podcast Category
  • hdVideo: iTunes Video Quality (2=Full HD, 1=HD, 0=SD)
  • keywords: Podcast Keywords
  • movement: Movement
  • movementIndex: Movement Index/Total, e.g. {no: 1, of: 2}
  • movementTotal: The total number of movements
  • podcastId: Podcast Identifier
  • showMovement: Show Movement
  • stik: iTunes Media Type (1=Normal, 2=Audiobook, 6=Music Video, 9=Movie, 10=TV Show, 11=Booklet, 14=Ringtone https://github.com/sergiomb2/libmp4v2/wiki/iTunesMetadata#user-content-media-type-stik
  • Make subtitle as multiple to match return type string[] (May a breaking change)
  • Add missing description type
  • Make compilation and podcast boolean type (string before) (May a breaking change)
  • Ignore generated test/samples/zeroes.flac
  • metadata.common.composersort has been changed to a singleton (May a breaking change)

ID3:

  • Set ID3v22 as CaseInsensitiveTagMap, ID3v24 is already (May a breaking change)
  • Add mapping MVNM to movement
  • Add mapping MVIN (1 field) to movementIndex
  • Add mapping PCS and PCST to podcast (Already exists for MP4)
  • Add mapping TCAT to category
  • Add mapping TCP to compilation (Already exists for ID3>2.2 and for MP4)
  • Add mapping TDES to description
  • Add mapping TDR and TDRL to date (Already exists for MP4)
  • Add mapping TGID to podcastId
  • Add mapping TKWD to keywords
  • Add mapping TS2 to albumartistsort (Already exists for ID3>2.2 and for MP4)
  • Add mapping TSA to albumsort (Already exists for ID3>2.2 and for MP4)
  • Add mapping TSC to composersort (Already exists for ID3>2.2 and for MP4)
  • Add mapping TSP to artistsort (Already exists for ID3>2.2 and for MP4)
  • Add mapping TST to titlesort (Already exists for ID3>2.2 and for MP4)
  • Add mapping WFD and WFED to podcasturl (Already exists for MP4)
  • Change mapping TOT from work to originalalbum (May a breaking change)
  • The follow fields seems not to exists like in MP4:
    • hdVideo
    • showMovement
    • stik
    • work

MP4:

  • Set as CaseInsensitiveTagMap (May a breaking change)
  • Add mapping ©com to comment
  • Add mapping ©cpy to copyright
  • Add mapping ©mvn to movement
  • Add mapping ©mvi/©mvc (2 fields) to movementIndex and movementTotal
  • Add mapping ©wrk to work
  • Add mapping catg to category
  • Add mapping egid to podcastId
  • Add mapping hdvd to hdVideo
  • Add mapping keyw to keywords
  • Add mapping shwm to showMovement
  • Add mapping stik to stik
  • Change mapping desc from description to subtitle for differentiate with ldesc (May a breaking change)

@Borewit Borewit assigned Borewit and ghost Jun 22, 2020
@Borewit Borewit self-requested a review June 22, 2020 07:45
Copy link
Owner

@Borewit Borewit left a comment

Choose a reason for hiding this comment

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

Very nice work, thank you so much for you contribution.

This is a significant change. Please describe the new (common, generic) tags which are added with this PR. Describe what they are used for, maybe what the originating (native) tags are. Link to the original tag documentation where possible.

lib/common/GenericTagTypes.ts Outdated Show resolved Hide resolved
lib/mp4/MP4Parser.ts Outdated Show resolved Hide resolved
@Borewit
Copy link
Owner

Borewit commented Jun 24, 2020

Thanks you so much for the documentation, that looks very good @ozelot379 .
I need to find some time review the PR in detail.

There are some small lint errors, you could possibly fix them in the mean time (yarn run lint).

@coveralls
Copy link

coveralls commented Jun 24, 2020

Coverage Status

Coverage increased (+0.002%) to 92.531% when pulling 65ef197 on ozelot379:add_some_mappings into 5d3c37f on Borewit:master.

Copy link
Owner

@Borewit Borewit left a comment

Choose a reason for hiding this comment

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

Can we add unit test demonstrating these mappings?

doc-gen/common.json Outdated Show resolved Hide resolved
doc-gen/common.json Outdated Show resolved Hide resolved
lib/id3v2/ID3v24TagMapper.ts Outdated Show resolved Hide resolved
lib/common/GenericTagTypes.ts Outdated Show resolved Hide resolved
lib/common/MetadataCollector.ts Outdated Show resolved Hide resolved
lib/mp4/MP4TagMapper.ts Outdated Show resolved Hide resolved
@ghost ghost changed the title Add some mappings Add and fix some mappings Jun 29, 2020
test/test-pr-544.ts Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Jun 29, 2020

@Borewit
I added an unit test.

But I had to change the follow too to make it work:

  • Make subtitle as multiple to match return type string[]
  • Add missing description type
  • Make compilation and podcast boolean type (string before)

I think it's a small breaking change, but at least the other unit tests are not broken

@Borewit
Copy link
Owner

Borewit commented Jun 29, 2020

@ozelot379. I think there is room for improvement in the unit tests. I have no problem with a reference to the original issue, but the unit test should describe which feature it is testing.

But this is all absolutely awesome work @ozelot379.

@ghost
Copy link
Author

ghost commented Jun 29, 2020

@Borewit
If I run build or test command, it generates some changes in doc/common_metadata.md and generates a new file test/samples/zeroes.flac

Is this correct and should I commit these files too?

@Borewit
Copy link
Owner

Borewit commented Jun 30, 2020

@Borewit
If I run build or test command, it generates some changes in doc/common_metadata.md and generates a new file test/samples/zeroes.flac

Is this correct and should I commit these files too?

doc/common_metadata.md, yes please, check it in. It is used to generate documentation.
test/samples/zeroes.flac, should be ignored.

@ghost ghost changed the title Add and fix some mappings Add, change and fix some mappings Jul 1, 2020
@Borewit Borewit merged commit 97d0f9a into Borewit:master Jul 6, 2020
@Borewit Borewit added the API change Enhancement or change to the current API label Jul 6, 2020
@Borewit
Copy link
Owner

Borewit commented Jul 6, 2020

Part of v7.0.0

@ghost ghost deleted the add_some_mappings branch July 6, 2020 15:45
@ghost
Copy link
Author

ghost commented Jul 6, 2020

🚀

@ghost ghost mentioned this pull request Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Enhancement or change to the current API enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants