Skip to content
This repository has been archived by the owner on Oct 4, 2024. It is now read-only.

Add color depth parameter to SpatialVideoMerger #12

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cbusillo
Copy link

The color depth parameter has been added to the SpatialVideoMerger constructors and used to determine the profile level in SpatialVideoMerger. This option is also added to the SpatialMediaKitTool for user input. The default value for color depth has been set as 8.

The color depth parameter has been added to the SpatialVideoMerger constructors and used to determine the profile level in SpatialVideoMerger. This option is also added to the SpatialMediaKitTool for user input. The default value for color depth has been set as 8.
@cbusillo cbusillo marked this pull request as ready for review June 19, 2024 20:37
@cbusillo
Copy link
Author

Hi, this is in reference to #11. This is my first time using Xcode, so please let me know if I messed anything up.

@@ -62,6 +62,8 @@ OPTIONS:
-r, --right-file <right-file>
The right eye media file to merge.
-q, --quality <quality> Output video quality [0-100]. 50 is a good default value.
-c, --color-depth <color-depth>
Copy link
Owner

Choose a reason for hiding this comment

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

I think this would make more sense as a boolean parameter, something like "--force-8-bit-color".

Copy link
Author

Choose a reason for hiding this comment

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

Now that you mention it, a boolean makes much more sense. I think the force is just a bit misleading though. I believe without that parameter it will ALWAYS be 10bit. As far as I could tell, it is not trying to detect 8/10 bit. Is starting with parameter with a number bad practice or even allowed? Can you think of a better name?

Copy link
Owner

@sturmen sturmen Jul 14, 2024

Choose a reason for hiding this comment

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

By convention, there's only a few verbs that are used with boolean flags. Perhaps the better name would be --disable-10-bit-color? My bias is towards 10-bit color by default, as the original intention of this tool was for combining professional intermediate codecs like ProRes, which are always 10-bit.

Copy link
Author

Choose a reason for hiding this comment

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

I like it! Sorry I didn't have enough experience to think about the Boolean flag and naming convention. All of the 3D Blu-ray sources I have encountered are 8 bit. Probably due to the standards being older.

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.

2 participants