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

Split box_types.go into multiple files #139

Closed
aler9 opened this issue Aug 1, 2023 · 2 comments
Closed

Split box_types.go into multiple files #139

aler9 opened this issue Aug 1, 2023 · 2 comments
Assignees

Comments

@aler9
Copy link
Contributor

aler9 commented Aug 1, 2023

Hello @sunfish-shogi, i'm about to contribute AV1 boxes, but i have a style-related doubt: the box_types.go file is getting huge and it's about to become even bigger in the future. This adds a degree of difficulty to all maintenance operations, i.e. finding boxes, adding boxes and keeping everything ordered.

In my opinion, the file should be splitted into multiple ones by using some criterion, for instance:

  • by specification (my choice), for instance:

    box_types_iso_14496_12.go
    box_types_iso_14496_14.go
    box_types_metadata.go
    box_types_other.go

  • by starting letter:

    box_types_a.go
    box_types_b.go

  • by single box. This would be optimal and doesn't cause any performance issue in go (see my library gomavlib) but it may be better to perform the operation in a dedicated folder, and this would break compatibility with the existing API:

    box_type_btrt.go
    box_type_colr.go

Regardless of the criterion, this is in my opinion a good way to improve maintainability of the library.

@sunfish-shogi
Copy link
Contributor

@aler9
Thank you for your suggestion!

by specification (my choice), for instance:
box_types_iso_14496_12.go
box_types_iso_14496_14.go
box_types_metadata.go
box_types_other.go

I think by-specification division rule is good.
I will create a pull request for that.

As a side note, go-mp4 had initially by-single-box files in root directory.

sunfish-shogi added a commit that referenced this issue Aug 2, 2023
Divide box_types.go by specification #139
@aler9
Copy link
Contributor Author

aler9 commented Aug 5, 2023

Fixed by #140

@aler9 aler9 closed this as completed Aug 5, 2023
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

No branches or pull requests

2 participants