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.

Sign up for GitHub

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

adding video compression (h264) #499

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

adding video compression (h264) #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f90b86e
adding video compression (h264)
bsobhani Dec 13, 2023
aee675e
add null compress/decompress and null config functions for when WITH_…
bsobhani Feb 26, 2024
560dda6
uncomment lock/unlock
bsobhani Mar 12, 2024
5b4623e
make function calls to H264 compress functions take context pointer
bsobhani Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ADApp/ADSrc/Codec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@ static std::string codecName[] = {
"jpeg",
"blosc",
"lz4",
"bslz4"
"bslz4",
"h264"
};

typedef enum {
NDCODEC_NONE,
NDCODEC_JPEG,
NDCODEC_BLOSC,
NDCODEC_LZ4,
NDCODEC_BSLZ4
NDCODEC_BSLZ4,
NDCODEC_H264
} NDCodecCompressor_t;

typedef struct Codec_t {
Expand Down
20 changes: 20 additions & 0 deletions ADApp/Db/NDCodec.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ record(mbbo, "$(P)$(R)Compressor")
field(THVL, "3")
field(FRST, "BSLZ4")
field(FRVL, "4")
field(FVST, "H264")
field(FVVL, "5")
info(autosaveFields, "VAL")
}

Expand All @@ -61,6 +63,8 @@ record(mbbi, "$(P)$(R)Compressor_RBV")
field(THVL, "3")
field(FRST, "BSLZ4")
field(FRVL, "4")
field(FVST, "H264")
field(FVVL, "5")
field(SCAN, "I/O Intr")
}

Expand Down Expand Up @@ -217,3 +221,19 @@ record(waveform, "$(P)$(R)CodecError")
field(FTVL, "CHAR")
field(NELM, "256")
}

# Sets group of picture size. A value of 1 is effectively frame-level compression. Set to higher values for true video compression.
record(ao, "$(P)$(R)GOPSize")
{
field(DTYP, "asynInt32")
field(OUT, "@asyn($(PORT),$(ADDR=0),$(TIMEOUT=1))GOP_SIZE")
}

# Sets the quantizer (qmin and qmax) to value written to PV.
# One can effectively adjust quality this way, with 1 being the least lossy, and higher numbers lossier
record(ao, "$(P)$(R)QMinMax")
{
field(DTYP, "asynInt32")
field(OUT, "@asyn($(PORT),$(ADDR=0),$(TIMEOUT=1))QMINMAX")
}
Comment on lines +232 to +238
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not allow users to set qmax and qmin separately?

Copy link
Author

Choose a reason for hiding this comment

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

It is my understanding that QMin and QMax set a range for acceptable "Q" (which stands for quantizer and roughly corresponds to quality), and then the codec selects a "Q" within this range based on various other factors. When I was testing this I found it useful to have a single PV that changes the compression quality, rather than separate min/max that must be set every time. It would be more complete to have separate QMin/QMax PVs, and then perhaps a PV that controls a "QCenter" which shifts QMin and QMax by the same amount as a convenience, although there is a little bit of complexity in implementing "QCenter", and I tried to keep things simple for the initial implementation. I agree that in principle QMin and QMax should be exposed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough, that's a more complicated interface.


14 changes: 14 additions & 0 deletions ADApp/commonDriverMakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,20 @@ ifeq ($(WITH_BLOSC),YES)
endif
endif

ifeq ($(WITH_VC),YES)
ifeq ($(VC_EXTERNAL),NO)
PROD_LIBS += videoCompression
else
ifdef VC_LIB
VC_DIR = $(VC_LIB)
PROD_LIBS += videoCompression
else
PROD_SYS_LIBS += videoCompression
endif
endif
endif
Comment on lines +163 to +174
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is relevant here and for your PR in ADSupport. This videoCompression library seems to only be implemented in ADSupport. Is there a chance it could even be provided externally?

On a similar vein, if it isn't ever external, I don't think VC_LIB is required.

Copy link
Author

Choose a reason for hiding this comment

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

Good point.



ifeq ($(WITH_SZIP),YES)
ifeq ($(SZIP_EXTERNAL),NO)
PROD_LIBS += szip
Expand Down
14 changes: 14 additions & 0 deletions ADApp/commonLibraryMakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ ifeq ($(WITH_BLOSC),YES)
endif
endif

ifeq ($(WITH_VC),YES)
ifeq ($(VC_EXTERNAL),NO)
PROD_LIBS += videoCompression
else
ifdef VC_LIB
VC_DIR = $(VC_LIB)
PROD_LIBS += videoCompression
else
PROD_SYS_LIBS += videoCompression
endif
endif
endif


ifeq ($(WITH_SZIP),YES)
ifeq ($(SZIP_EXTERNAL),NO)
LIB_LIBS += szip
Expand Down
5 changes: 5 additions & 0 deletions ADApp/pluginSrc/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ ifeq ($(WITH_BITSHUFFLE), YES)
USR_CXXFLAGS += -DHAVE_BITSHUFFLE
endif

ifeq ($(WITH_VC), YES)
#INC += video_compression.h
USR_CXXFLAGS += -DHAVE_VC
endif

ifdef BLOSC_INCLUDE
USR_INCLUDES += $(addprefix -I, $(BLOSC_INCLUDE))
endif
Expand Down
112 changes: 111 additions & 1 deletion ADApp/pluginSrc/NDPluginCodec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,6 @@ NDArray *compressLZ4(NDArray *input, NDCodecStatus_t *status, char *errorMessage
return output;
}


NDArray *decompressLZ4(NDArray *input, NDCodecStatus_t *status, char *errorMessage)
{
// Sanity check
Expand Down Expand Up @@ -635,6 +634,97 @@ NDArray *decompressBSLZ4(NDArray *input, NDCodecStatus_t *status, char *errorMes

#endif // ifdef HAVE_BITSHUFFLE

#ifdef HAVE_VC
#include <video_compression.h>
//extern int H264_compress_default(const char*, char*, int, int, int);
NDArray *compressH264(void** vc_context, NDArray *input, NDCodecStatus_t *status, char *errorMessage)
{
//printf("inside compressH264\n");
if (!input->codec.empty()) {
sprintf(errorMessage, "Array is already compressed");
*status = NDCODEC_WARNING;
return NULL;
}

switch (input->dataType) {
case NDInt8:
case NDUInt8:
//case NDInt16:
//case NDUInt16:
break;
default:
sprintf(errorMessage, "H264 only supports 8-bit data");
*status = NDCODEC_ERROR;
//goto failure;
return NULL;
}



NDArrayInfo_t info;
input->getInfo(&info);
int outputSize = LZ4_compressBound((int)info.totalBytes);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is LZ4_compressBound the function that should be used here?

Copy link
Author

Choose a reason for hiding this comment

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

LZ4_compressBound appears to be a simple function that calculates a number slightly larger than the value that it is given. I found this sufficient when testing this plugin, but I will think about if another calculation (or perhaps simply using info.totalBytes) would be more suitable.

Copy link
Contributor

Choose a reason for hiding this comment

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

But LZ4_compressBound is also specific to LZ4 compression. That it works for video compression is more a coincidence than something that should be relied upon. That's why I think passing the buffer size to the compression function would at least allow us to catch issues with buffer size and avoid memory corruption. FFMpeg might also provide a function to help us determine the necessary buffer size, based on the configured bit-rate and quantization.

NDArray *output = allocArray(input, -1, outputSize);

if (!output) {
sprintf(errorMessage, "Failed to allocate H264 output array");
*status = NDCODEC_ERROR;
return NULL;
}

//int compSize = LZ4_compress_default((const char*)input->pData, (char*)output->pData, (int)info.totalBytes, outputSize);
int x_size = input->dims[0].size;
int y_size = input->dims[1].size;

//printf("x_size %d y_size %d\n", x_size, y_size);
//printf("before H264 compress default\n");
//int compSize = H264_compress_default((const char*)input->pData, (char*)output->pData, (int)x_size, (int)y_size, outputSize);
//int compSize = H264_compress((const char*)input->pData, (char*)output->pData, (int)x_size, (int)y_size);
int compSize = H264_compress(vc_context, (const char*)input->pData, (char*)output->pData, (int)x_size, (int)y_size);

if (compSize <= 0) {
output->release();
sprintf(errorMessage, "Internal H264 error");
*status = NDCODEC_ERROR;
return NULL;
}

output->codec.name = codecName[NDCODEC_H264];
output->compressedSize = compSize;
//printf("before return output\n");

return output;
}

#else

NDArray *compressH264(void** vc_context, NDArray *input, NDCodecStatus_t *status, char *errorMessage)
{
sprintf(errorMessage, "No H264 support");
*status = NDCODEC_ERROR;
return NULL;
}

NDArray *decompressH264(NDArray *input, NDCodecStatus_t *status, char *errorMessage)
{
sprintf(errorMessage, "No H264 support");
*status = NDCODEC_ERROR;
return NULL;
}


void set_gop_size(int value){
printf("unable to set gop size to %d\n", value);
}

void set_q_min_max(int value){
printf("unable to set q min max to %d\n", value);
}
Comment on lines +708 to +722
Copy link
Contributor

Choose a reason for hiding this comment

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

These functions are implemented only for the case where HAVE_VC isn't defined.

Copy link
Contributor

@ericonr ericonr Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see now that set_gop_size and set_q_min_max are defined in the ADSupport functions. They are operating on global status; is it safe to operate on them during compression, for example? And this means we can't have more than one compression stream on the same IOC.

Is that part of the reason why the compress function can't run with unlock() called before it?

Copy link
Author

Choose a reason for hiding this comment

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

Yes I think a global variable is not ideal because I believe it is possible to have two separate cameras in a single IOC, and hence multiple instances of the plugin. I did not think of this. In practice, there is usually only camera per IOC. I don't think it is likely that the global variable is the reason for the unlock issues since I did my testing with only one instance of the plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is likely that the global variable is the reason for the unlock issues since I did my testing with only one instance of the plugin.

Even with a single instance of the plugin, but without the additional locks you added, if you write into the QMinMax or GOPSize PVs while compression is happening, you will reset the codec state while the struct is being used. That's not safe. These functions should be implemented in NDPluginCodec, which can track the current encoding state, and block writes to these values if encoding is ongoing.


#endif



/** Callback function that is called by the NDArray driver with new NDArray data.
* Does JPEG or Blosc compression on the array.
* If compression is None or fails the input array is passed on without
Expand Down Expand Up @@ -717,6 +807,13 @@ void NDPluginCodec::processCallbacks(NDArray *pArray)
break;
}

case NDCODEC_H264: {
unlock();
result = compressH264(&vc_context, pArray, &codecStatus, errorMessage);
lock();
break;
}

}

if (result && result != pArray) {
Expand Down Expand Up @@ -807,8 +904,17 @@ asynStatus NDPluginCodec::writeInt32(asynUser *pasynUser, epicsInt32 value)
value = 1;
} else if (function < FIRST_NDCODEC_PARAM) {
status = NDPluginDriver::writeInt32(pasynUser, value);
} else if (function == NDCodecGOPSize) {
printf("setting GOP Size...\n");
set_gop_size(vc_context, value);
} else if (function == NDCodecQMinMax) {
//lock();
printf("setting qmin and qmax...\n");
set_q_min_max(vc_context, value);
//unlock();
}


/* Set the parameter in the parameter library. */
status = (asynStatus) setIntegerParam(function, value);

Expand Down Expand Up @@ -877,6 +983,8 @@ NDPluginCodec::NDPluginCodec(const char *portName, int queueSize, int blockingCa
createParam(NDCodecBloscCLevelString, asynParamInt32, &NDCodecBloscCLevel);
createParam(NDCodecBloscShuffleString, asynParamInt32, &NDCodecBloscShuffle);
createParam(NDCodecBloscNumThreadsString, asynParamInt32, &NDCodecBloscNumThreads);
createParam(NDCodecGOPSizeString, asynParamInt32, &NDCodecGOPSize);
createParam(NDCodecQMinMaxString, asynParamInt32, &NDCodecQMinMax);
Comment on lines +986 to +987
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't following the existing formatting.


/* Set the plugin type string */
setStringParam(NDPluginDriverPluginType, "NDPluginCodec");
Expand All @@ -894,6 +1002,8 @@ NDPluginCodec::NDPluginCodec(const char *portName, int queueSize, int blockingCa
// This plugin currently ignores this setting and always does callbacks, so make the setting reflect the behavior
setIntegerParam(NDArrayCallbacks, 1);

void* vc_context=0;

/* Try to connect to the array port */
connectToArrayPort();
}
Expand Down
5 changes: 5 additions & 0 deletions ADApp/pluginSrc/NDPluginCodec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#define NDCodecBloscCLevelString "BLOSC_CLEVEL" /* (int r/w) Blosc compression level */
#define NDCodecBloscShuffleString "BLOSC_SHUFFLE" /* (bool r/w) Should Blosc apply shuffling? */
#define NDCodecBloscNumThreadsString "BLOSC_NUMTHREADS" /* (int r/w) Number of threads to be used by Blosc */
#define NDCodecGOPSizeString "GOP_SIZE" /* (int r/w) Group of pictures size for H264 */
#define NDCodecQMinMaxString "QMINMAX" /* (int r/w) Sets min and max values for quantizer for H264 */
Comment on lines +16 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't following the existing formatting either.


/** Compress/decompress NDArrays according to available codecs.
* This plugin is a source of NDArray callbacks, passing the (possibly
Expand Down Expand Up @@ -74,6 +76,7 @@ class NDPLUGIN_API NDPluginCodec : public NDPluginDriver {
/* These methods override the virtual methods in the base class */
void processCallbacks(NDArray *pArray);
asynStatus writeInt32(asynUser *pasynUser, epicsInt32 value);
void* vc_context;

protected:
int NDCodecMode;
Expand All @@ -87,6 +90,8 @@ class NDPLUGIN_API NDPluginCodec : public NDPluginDriver {
int NDCodecBloscCLevel;
int NDCodecBloscShuffle;
int NDCodecBloscNumThreads;
int NDCodecGOPSize;
int NDCodecQMinMax;

};

Expand Down