Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Add AudioSimple asset #360

Closed
wants to merge 4 commits into from

Conversation

robertlong
Copy link

This PR contains a minimal sample asset for testing the draft KHR_audio extension.

It includes the source model and a glTF + glb version of the asset. The model was generated in Blender and the audio extension was added by hand. The glb was compiled using Third Room's implementation of glTF-transform.


## Screenshot

![screenshot](screenshot/screenshot.jpg)
Copy link
Contributor

@cx20 cx20 Oct 8, 2022

Choose a reason for hiding this comment

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

Since the file extensions do not match, you probably need to change the extension of the referenced file to .png.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think the size of screenshot.png is bigger than the others. can you change 1200x1200 to about 400x400?
This is because screenshot.png is also used for list display.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cx20 On the one hand, I agree. On the other hand, it's a bit subtle:

  • One possible reason to use smaller screenshots is to not bloat the size of the repository. This particularly refers to samples where the visual presentation is not important (like this one): There is no visual detail that is relevant in the screenshot. One could argue about that for other models, where the screenshot shows "a nice rendering result"
  • The size of the screenshot affects the column in the table in a non-obvious way. Markdown does not explicitly allow setting the column width for a table. And the GitHub markdown preview apparently does some sort of "weighting": The screenshot here is 1200 pixels wide, but the table column is not - it is only "relatively wide" compared to the other columns...

Subjectively, I like such screenshots and their width to be a bit more "regular" and "predictable". For example:

The latter can (unfortunately) only be achieved with some (non-markdown) <img...> tags. But I think that it looks nicer this way...

Copy link
Contributor

Choose a reason for hiding this comment

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

(It's a detail, but I just brought this up in https://github.com/KhronosGroup/glTF-Sample-Models/issues/362 nevertheless...)

Copy link
Author

@robertlong robertlong Nov 10, 2022

Choose a reason for hiding this comment

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

What's the final decision on this? I fixed the file extension, should I still resize the thumbnail?

Copy link
Contributor

Choose a reason for hiding this comment

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

@robertlong Thank you for correcting the file extension.
I think it would be nice to have a small image for the thumbnail, but I'm not too particular about it.

BTW, Regarding the screenshot, how about making it a forest rather than the sea?
The reason I think so is because the environmental sounds are the chirping of birds.

Comment on lines +79 to +85
"mesh": 1,
"name": "Sphere",
"scale": [0.5, 0.5, 0.5],
"translation": [5.605831146240234, 1.0160478353500366, 0],
"extensions": {
"KHR_audio": {
"emitter": 1
Copy link

@aaronfranke aaronfranke May 4, 2023

Choose a reason for hiding this comment

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

This node has both an audio emitter and a Mesh. This significantly complicates import into Godot Engine which only allows one type per node, since we would need to import 2 nodes in this case.

Instead, I would be in favor of modifying the KHR_audio standard to only allow audio emitters on nodes that aren't meshes (or cameras, lights, physics nodes, etc). Even aside from implementation difficulty, it makes more sense to have these be separate, so that an object's audio transform is not necessarily locked to the mesh's transform.

@DRx3D
Copy link
Contributor

DRx3D commented Nov 25, 2023

There is no ratified Audio extension. This PR will be closed without merge by 29 Nov. Future PRs should only use ratified extensions and be against Sample-Assets repo.

@DRx3D
Copy link
Contributor

DRx3D commented Dec 3, 2023

PR closed because there is no ratified extension. A new PR can be opened in glTF-Sample-Assets when an extension reaches 'release candidate' status.

@DRx3D DRx3D closed this Dec 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants