gopro / cineform-sdk

The GoPro® CineForm video codec SDK.
Apache License 2.0
275 stars 57 forks source link

LUTPaths: when is it used/useful? #16

Open shekh opened 6 years ago

shekh commented 6 years ago

There is some weirdness here (hardcoded fully qualified path) https://github.com/gopro/cineform-sdk/blob/1eb2cb7914c38db128b9518ab25dd9f3f24c9890/Codec/lutpath.cpp#L483

and here https://github.com/gopro/cineform-sdk/blob/1eb2cb7914c38db128b9518ab25dd9f3f24c9890/DecoderSDK/SampleMetadata.cpp#L62

Is this actually executed at some point? Also there are registry access, system variable access etc. Is it a convention shared between all CineForm-enabled programs? Which programs create these registry keys and lut files?

retokromer commented 6 years ago

I’m also interested in knowing more about the use of LUTs in CineForm.

dnewman-gpsw commented 6 years ago

Active Metadata is runs out of this path. CineForm FirstLight and GoPro Studio uses these for color correction and other visual enhancements that occur during decode. Also the defaults paths are typically overridden with system environment controls.

dnewman-gpsw commented 6 years ago

I just noted that Active Metadata was not be supported within VDFM, some CineForm user will miss this (including myself).

shekh commented 6 years ago

Good to know. I planned to collect some feedback and then have second attempt.

What do you think about replacing "C:\Program Files\Common Files\" with SHGetSpecialFolderPathW(CSIDL_PROGRAM_FILES_COMMON)? Leaving it as is leads to undefined behavior where c: drive does not exist, is removable, remote etc.

Also related somehow.. Is anyone having trouble compiling with atlbase/ccombstr dependency? It seems quite unnecessary, I`d love to see it go away (please somebody replace it with std::wstring :)

shekh commented 6 years ago

Ok, atlbase can be removed with no loss by adding this fragment to settings.h

#include <tchar.h>

class CComBSTR
{
public:
    char* str;

    CComBSTR()
    {
        str = NULL;
    }

    CComBSTR(LPCSTR pSrc)
    {
        str = NULL;
        if(pSrc){
            int size = strlen(pSrc);
            str = (char*)malloc(size+1);
            strcpy(str,pSrc);
        }
    }

    ~CComBSTR()
    {
        free(str);
    }
};

#define USES_CONVERSION

inline char* OLE2T(CComBSTR& a){ return a.str; }
dnewman-gpsw commented 6 years ago

Cool. I will try that out.

dnewman-gpsw commented 6 years ago

This didn't work for me. So I cleaned up the code a different way. I have removed settings.cpp/h and atlbase.h from the Windows builds.

shekh commented 6 years ago

Great. Now it looks like it does not need the registry key to exist (SOFTWARE\CineForm\ColorProcessing) Is it still good idea to think: if there is any AM db the key also exists and points to db? In previous code if(cfg.Open...) looked like the intent was to check for key existence.

dnewman-gpsw commented 6 years ago

An existing CineForm users will have these setup via the registry keys. New users can run with the defaults as they have not disk based database of active metadata changes. Users can manipulate the LUTPaths and DBPaths for interesting workflows like this https://vimeo.com/10024749

zcream commented 4 years ago

Has anyone implemented metadata support outside of first light?