On 21/03/16 12:29, Diego Biurrun wrote:
> On Mon, Mar 21, 2016 at 12:04:23AM +0000, Mark Thompson wrote:
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -105,7 +105,7 @@ OBJS-$(CONFIG_STARTCODE) += startcode.o
>> OBJS-$(CONFIG_TEXTUREDSP) += texturedsp.o
>> OBJS-$(CONFIG_TEXTUREDSPENC) += texturedspenc.o
>> OBJS-$(CONFIG_TPELDSP) += tpeldsp.o
>> -OBJS-$(CONFIG_VAAPI_ENCODE) += vaapi_encode.o
>> +OBJS-$(CONFIG_VAAPI_ENCODE) += vaapi_encode.o vaapi_encode_h26x.o
>
> Is vaapi_encode_h26x.o required by your MJPEG encoder as well? If not,
> then this object file should not be compiled for it. You can add
> another vaapi_encode_h26x component that conditionally enables it.
It only applies to H.264 and H.265.
Would it be better to instead add the vaapi_encode_h26x.o object to the lists
for both of the other encoders?
>> --- /dev/null
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -0,0 +1,817 @@
>> +
>> +typedef struct {
>> +} VAAPIEncodeH264MiscSliceParams;
>
> Please avoid anonymously typedeffed structs.
You mean just give it a tag, so:
typedef struct VAAPIEncodeH264MiscSliceParams {
...
} VAAPIEncodeH264MiscSliceParams;
?
>> --- /dev/null
>> +++ b/libavcodec/vaapi_encode_h26x.h
>> @@ -0,0 +1,70 @@
>> +
>> +// Set to 1 to log a full trace of all bitstream output (debugging only).
>> +#if 0
>
> Ugh, commented out code tends to rot and break... :-/
The trace code is useful for comparison against the decoder while debugging it;
it has little value once the code is all written.
>> +static void trace_write_u(PutBitContext *pbc, unsigned int width,
>> + unsigned int value, const char *name)
>
> Can't this be achieved through the TRACE loglevel?
I guess avoiding compiling it in is only a minor (and probably entirely
irrelevant) optimisation, so it could just stay in all the time. Only output
anything at TRACE, though.
>> +int ff_vaapi_encode_h26x_nal_unit_to_byte_stream(uint8_t *dst, size_t
>> *dst_len,
>> + uint8_t *src, size_t
>> src_len);
>
> This header should directly #include stdint.h and sys/types.h.
Or stdint.h and stddef.h?
Thanks,
- Mark
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
