[FFmpeg-devel] patch for a new H.264 codec

[Yufei He]

Hi Tomas

Thanks for the review.

My codec is a hardware codec, it depends an dynamic library that goes 
with the M264 card.

The way it works will be:

1>Our customers buy the card and install the drivers of it.

2>They Compile ffmpeg.

3>Call ffmpeg with codec option of m264.

That's why I use dlopen to load the dynamic library that talks with the 
hardware.

Please help us check if it works in ffmpeg.

Thanks.

Yufei.



On 02/25/2019 03:45 PM, Tomas Härdin wrote:
> Hi
>
>> +void convert_to_annexb(unsigned char * dest, unsigned long
>> data_size)
>> +{
>> +   unsigned char *current = dest;
>> +   union
>> +   {
>> +      unsigned char by4[4];
>> +      uint32_t length;
>> +   } union_u32_byte;
>> +
>> +   while ((dest + data_size) > (current + 4))
>> +   {
>> +      if((current[0] == 0) &&
>> +         (current[1] == 0) &&
>> +         (current[2] == 0) &&
>> +         (current[3] == 1))
>> +      {
>> +         // in case it is already in annex B
>> +         break;
>> +      }
>> +
>> +      union_u32_byte.by4[3] = current[0];
>> +      union_u32_byte.by4[2] = current[1];
>> +      union_u32_byte.by4[1] = current[2];
>> +      union_u32_byte.by4[0] = current[3];
> This won't work on certain machines. Type punning is also undefined if
> memory serves me right, and may not work right in all compilers.
> Definitely not recommended. There are macros in FFmpeg for this already
> (AV_RL32 and friends)
>
>> +#ifdef _WIN32
>> +   lib_handle = dlopen("mvM264Ffmpeg.dll", RTLD_LAZY);
>> +#else
>> +   lib_handle = dlopen("libmvM264Ffmpeg.so", RTLD_LAZY);
>> +#endif
> I believe FFmpeg has policy specifically against stuff like this. Link
> the dynamic library properly, and put appropriate license stuff in
> configure. The resulting ffmpeg binaries will more than likely not be
> redistributable, libav* might also be depending on what the Matrox
> library's license is.
>
>> +   printf("m264_encode_init_h264: avctx->width = %d\n", avctx->width);
>> +   printf("m264_encode_init_h264: avctx->height = %d\n", avctx->height);
>> +   printf("m264_encode_init_h264: avctx->framerate.num = %d\n", avctx->framerate.num);
>> +   printf("m264_encode_init_h264: avctx->framerate.den = %d\n", avctx->framerate.den);
>> +   printf("m264_encode_init_h264: avctx->gop_size = %d\n", avctx->gop_size);
>> +   printf("m264_encode_init_h264: avctx->bit_rate = %" PRId64 "\n", avctx->bit_rate);
> Forgotten debug stuff? printf() is definitely not OK
>
> There are other issues with this patch, but there's no point in looking
> more for now
>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel